Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cover WTG1001 on edge cases #225

Merged

Conversation

mj-wtg
Copy link
Contributor

@mj-wtg mj-wtg commented Jul 8, 2024

Fix #224
Tracked by WI00764889

  • A method with default access modifier, partial, return value
    e.g. private partial int Bar(); // no error as it returns value

  • A partial method with non-default access modifier, void
    e.g. public partial void Bar(); // no error as it returns value

  • A partial method with default access modifier, void
    e.g. private partial void Bar(); // trigger WTG1001 and remove 'private' when code-fix run

  • A partial method with default access modifier, void, out value
    e.g. private partial void Bar(out int value); // no error as it 'out' a value

  • A partial method with default access modifier, return value, out value
    e.g. private partial int Bar(out int value); // no error as it returns and 'out' a value

- apply WTG1001 fix that removes 'private' keyword if the method is 'partial' and 'void'
- keep 'private' keyword if a partial method is non-void or having 'out' keyword
@@ -47,11 +50,10 @@ static void Analyze(SyntaxNodeAnalysisContext context, FileDetailCache cache)

case SyntaxKind.ProtectedKeyword:
case SyntaxKind.PublicKeyword:
case SyntaxKind.PartialKeyword when (context.Node.IsKind(SyntaxKind.MethodDeclaration)):
case SyntaxKind.PartialKeyword when (DoesMethodHaveReturnValueOrOutKeyword(currentNode)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
case SyntaxKind.PartialKeyword when (DoesMethodHaveReturnValueOrOutKeyword(currentNode)):
case SyntaxKind.PartialKeyword when (PartialMethodRequiresAccessibilityModifier(currentNode)):

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll apply the name as suggested.

}

var methodNode = (MethodDeclarationSyntax)node;
if (methodNode.ReturnType is PredefinedTypeSyntax predefinedTypeSyntax
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a Kind check we can perform on methodNode.ReturnType before turning to runtime type casts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose the same as Kind check for var methodNode = (MethodDeclarationSyntax)node;, e.g.

if (methodNode.ReturnType.IsKind(SyntaxKind.PredefinedType))
{
	var returnType = (PredefinedTypeSyntax)methodNode.ReturnType;
	if (!returnType.Keyword.IsKind(SyntaxKind.VoidKeyword))
	{
		return true;
	}
}

return true;
}

if (methodNode.ParameterList?.Parameters.Any(c => c.Modifiers.Any(SyntaxKind.OutKeyword)) == true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (methodNode.ParameterList?.Parameters.Any(c => c.Modifiers.Any(SyntaxKind.OutKeyword)) == true)
if (methodNode.ParameterList?.Parameters.Any(static c => c.Modifiers.Any(SyntaxKind.OutKeyword)) == true)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll apply this.

}

partial class Foo
{
private partial int Bar() { return default; }
public partial void FooBar() { }
private partial int FooBarBaz(out int value) { value = default; return default; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option for methods that we don't care about:

Suggested change
private partial int FooBarBaz(out int value) { value = default; return default; }
private partial int FooBarBaz(out int value) => throw null;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll apply this.

@mj-wtg mj-wtg requested a review from yaakov-h July 12, 2024 02:06
@mj-wtg mj-wtg merged commit 7ab99e1 into master Aug 5, 2024
7 checks passed
@mj-wtg mj-wtg deleted the MNJ/WI00764889/WTG1001_Exclude_NonVoid_Partial_Method branch August 5, 2024 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants