Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.Extensions;
using Microsoft.CodeAnalysis.CSharp.LanguageService;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Operations;
using Microsoft.CodeAnalysis.RemoveUnusedParametersAndValues;

Expand All @@ -33,6 +35,22 @@ protected override bool MethodHasHandlesClause(IMethodSymbol method)
protected override bool IsIfConditionalDirective(SyntaxNode node)
=> node is IfDirectiveTriviaSyntax;

protected override bool ReturnsThrow(SyntaxNode node)
{
var methodSyntax = (MethodDeclarationSyntax)node;
if (methodSyntax.ExpressionBody is not null)
{
return methodSyntax.ExpressionBody.Expression is ThrowExpressionSyntax;
}

if (methodSyntax.Body is not null && methodSyntax.Body.Statements.Count == 1)
{
return methodSyntax.Body.Statements[0] is ThrowStatementSyntax;
}

return false;
}

protected override CodeStyleOption2<UnusedValuePreference> GetUnusedValueExpressionStatementOption(AnalyzerOptionsProvider provider)
=> ((CSharpAnalyzerOptionsProvider)provider).UnusedValueExpressionStatement;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1647,5 +1647,28 @@ public static ICustomMarshaler GetInstance(string [|s|])
}
");
}

[Fact, WorkItem(47174, "https://github.com/dotnet/roslyn/issues/65275")]
public async Task TestMethodWithUnusedParameterThrowsExpressionBOdy()
{
await TestDiagnosticMissingAsync(
@"public class Class
{
public void Method(int [|x|]) => throw new System.Exception();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a test that makes sure it still reports if there's more than just a throw?

}");
}

[Fact, WorkItem(47174, "https://github.com/dotnet/roslyn/issues/65275")]
public async Task TestMethodWithUnusedParameterThrowsMethodBody()
{
await TestDiagnosticMissingAsync(
@"public class Class
{
public void Method(int [|x|])
{
throw new System.Exception();
}
}");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.FlowAnalysis.SymbolUsageAnalysis;
using Microsoft.CodeAnalysis.LanguageService;
Copy link
Contributor

Choose a reason for hiding this comment

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

needed?

using Microsoft.CodeAnalysis.Operations;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,9 @@ parameter.ContainingSymbol is not IMethodSymbol method ||

// Ignore parameters of record primary constructors since they map to public properties
// TODO: Remove this when implicit operations are synthesised: https://github.com/dotnet/roslyn/issues/47829
var methodSyntax = method.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax();
if (method.IsConstructor() &&
_compilationAnalyzer.IsRecordDeclaration(method.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax()))
_compilationAnalyzer.IsRecordDeclaration(methodSyntax))
{
return false;
}
Expand Down Expand Up @@ -270,6 +271,11 @@ parameter.ContainingSymbol is not IMethodSymbol method ||
return false;
}

if (_compilationAnalyzer.ReturnsThrow(methodSyntax))
{
return false;
}

// Don't report on valid GetInstance method of ICustomMarshaler.
// See https://docs.microsoft.com/dotnet/api/system.runtime.interopservices.icustommarshaler#implementing-the-getinstance-method
if (method is { MetadataName: "GetInstance", IsStatic: true, Parameters.Length: 1, ContainingType: { } containingType } methodSymbol &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ protected AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer(
protected abstract bool SupportsDiscard(SyntaxTree tree);
protected abstract bool MethodHasHandlesClause(IMethodSymbol method);
protected abstract bool IsIfConditionalDirective(SyntaxNode node);
protected abstract bool ReturnsThrow(SyntaxNode node);
protected abstract CodeStyleOption2<UnusedValuePreference> GetUnusedValueExpressionStatementOption(AnalyzerOptionsProvider provider);
protected abstract CodeStyleOption2<UnusedValuePreference> GetUnusedValueAssignmentOption(AnalyzerOptionsProvider provider);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Imports Microsoft.CodeAnalysis.Diagnostics
Imports Microsoft.CodeAnalysis.Operations
Imports Microsoft.CodeAnalysis.RemoveUnusedParametersAndValues
Imports Microsoft.CodeAnalysis.VisualBasic.CodeStyle
Imports Microsoft.CodeAnalysis.VisualBasic.LanguageService
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax

Namespace Microsoft.CodeAnalysis.VisualBasic.RemoveUnusedParametersAndValues
Expand Down Expand Up @@ -50,6 +51,22 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.RemoveUnusedParametersAndValues
Return TryCast(expressionStatement.Syntax, CallStatementSyntax) IsNot Nothing
End Function

Protected Overrides Function ReturnsThrow(node As SyntaxNode) As Boolean
Dim methodStatementSyntax = TryCast(node, MethodStatementSyntax)
If methodStatementSyntax IsNot Nothing Then
Dim methodSyntax = TryCast(node.Parent, MethodBlockSyntax)
If methodSyntax.BlockStatement Is Nothing Then
Return False
End If

If methodSyntax.Statements.Count = 1 Then
Return TryCast(methodSyntax.Statements.First(), ThrowStatementSyntax) IsNot Nothing
End If
End If

Return False
End Function

Protected Overrides Function IsExpressionOfExpressionBody(expressionStatementOperation As IExpressionStatementOperation) As Boolean
' VB does not support expression body
Return False
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,18 @@ class C
if (true)
throw new NotImplementedException()
end sub
end class")
End Function

<Fact, WorkItem(41236, "https://github.com/dotnet/roslyn/issues/41236")>
Public Async Function TestMethodBody_OnlyThrowStatement() As Task
Await TestDiagnosticMissingAsync(
"imports system

class C
private sub Goo([|i|] as integer)
throw new Exception()
end sub
end class")
End Function
End Class
Expand Down