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.

Sign up for GitHub

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

Support VB for ImmutableObjectMethodAnalyzer #6321

Merged
merged 4 commits into from
Jan 4, 2023
Merged

Support VB for ImmutableObjectMethodAnalyzer #6321

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ad3be7b
Support VB for ImmutableObjectMethodAnalyzer
Youssef1313 Dec 10, 2022
10f4d82
Run pack
Youssef1313 Dec 10, 2022
c8ae325
Update ImmutableObjectMethodAnalyzer.cs
Youssef1313 Jan 3, 2023
c4909ec
Fix implementation
Youssef1313 Jan 3, 2023
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
6 changes: 6 additions & 0 deletions src/Microsoft.CodeAnalysis.Analyzers/CSharp/AnalyzerReleases.Unshipped.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
; Please do not edit this file manually, it should only be updated through code fix application.

### Removed Rules

Rule ID | Category | Severity | Notes
--------|----------|----------|-------
RS1014 | MicrosoftCodeAnalysisCorrectness | Warning | CSharpImmutableObjectMethodAnalyzer
111 changes: 0 additions & 111 deletions src/Microsoft.CodeAnalysis.Analyzers/CSharp/CSharpImmutableObjectMethodAnalyzer.cs
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/AnalyzerReleases.Unshipped.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@

Rule ID | Category | Severity | Notes
--------|----------|----------|-------
RS1014 | MicrosoftCodeAnalysisCorrectness | Warning | ImmutableObjectMethodAnalyzer
RS1035 | MicrosoftCodeAnalysisCorrectness | Error | SymbolIsBannedInAnalyzersAnalyzer
RS1036 | MicrosoftCodeAnalysisCorrectness | Warning | SymbolIsBannedInAnalyzersAnalyzer
2 changes: 1 addition & 1 deletion src/Microsoft.CodeAnalysis.Analyzers/Core/CodeAnalysisDiagnosticsResources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@
<value>'{0}' is immutable and '{1}' will not have any effect on it. Consider using the return value from '{1}'.</value>
</data>
<data name="DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle" xml:space="preserve">
<value>Do not ignore values returned by methods on immutable objects.</value>
<value>Do not ignore values returned by methods on immutable objects</value>
mavasani marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="DiagnosticIdMustBeAConstantTitle" xml:space="preserve">
<value>DiagnosticId for analyzers must be a non-null constant</value>
Expand Down
108 changes: 108 additions & 0 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/ImmutableObjectMethodAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using System.Linq;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.CodeAnalysis.Analyzers
{
using static CodeAnalysisDiagnosticsResources;

/// <summary>
/// RS1014: <inheritdoc cref="DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle"/>
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class ImmutableObjectMethodAnalyzer : DiagnosticAnalyzer
{
public static readonly DiagnosticDescriptor DoNotIgnoreReturnValueDiagnosticRule = new(
DiagnosticIds.DoNotIgnoreReturnValueOnImmutableObjectMethodInvocation,
CreateLocalizableResourceString(nameof(DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle)),
CreateLocalizableResourceString(nameof(DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationMessage)),
DiagnosticCategory.MicrosoftCodeAnalysisCorrectness,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: CreateLocalizableResourceString(nameof(DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationDescription)),
customTags: WellKnownDiagnosticTagsExtensions.Telemetry);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(DoNotIgnoreReturnValueDiagnosticRule);

private const string SolutionFullName = @"Microsoft.CodeAnalysis.Solution";
private const string ProjectFullName = @"Microsoft.CodeAnalysis.Project";
private const string DocumentFullName = @"Microsoft.CodeAnalysis.Document";
private const string SyntaxNodeFullName = @"Microsoft.CodeAnalysis.SyntaxNode";
private const string CompilationFullName = @"Microsoft.CodeAnalysis.Compilation";

private static readonly ImmutableArray<string> s_immutableMethodNames = ImmutableArray.Create(
"Add",
"Remove",
"Replace",
"With");

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);

context.RegisterCompilationStartAction(context =>
{
var compilation = context.Compilation;
var builder = ImmutableArray.CreateBuilder<INamedTypeSymbol>();
var provider = WellKnownTypeProvider.GetOrCreate(compilation);
AddIfNotNull(builder, provider.GetOrCreateTypeByMetadataName(SolutionFullName));
AddIfNotNull(builder, provider.GetOrCreateTypeByMetadataName(ProjectFullName));
AddIfNotNull(builder, provider.GetOrCreateTypeByMetadataName(DocumentFullName));
AddIfNotNull(builder, provider.GetOrCreateTypeByMetadataName(SyntaxNodeFullName));
AddIfNotNull(builder, provider.GetOrCreateTypeByMetadataName(CompilationFullName));
var immutableTypeSymbols = builder.ToImmutable();
if (immutableTypeSymbols.Length > 0)
{
context.RegisterOperationAction(context => AnalyzeInvocationForIgnoredReturnValue(context, immutableTypeSymbols), OperationKind.Invocation);
}
});

static void AddIfNotNull(ImmutableArray<INamedTypeSymbol>.Builder builder, INamedTypeSymbol? symbol)
{
if (symbol is not null)
{
builder.Add(symbol);
}
}
}

public static void AnalyzeInvocationForIgnoredReturnValue(OperationAnalysisContext context, ImmutableArray<INamedTypeSymbol> immutableTypeSymbols)
{
var invocation = (IInvocationOperation)context.Operation;
if (invocation.Parent is not IExpressionStatementOperation)
{
return;
}

// If the method doesn't start with something like "With" or "Replace", quit
string methodName = invocation.TargetMethod.Name;
if (!s_immutableMethodNames.Any(n => methodName.StartsWith(n, StringComparison.Ordinal)))
{
return;
}

INamedTypeSymbol? type = null;
if (invocation.Instance is not null)
{
type = invocation.Instance.Type as INamedTypeSymbol;
}
else if (invocation.TargetMethod.IsExtensionMethod && invocation.TargetMethod.Parameters is [{ Type: var parameterType }, ..])
{
type = parameterType as INamedTypeSymbol;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

@mavasani Does this logic sound correct? I'm surprised there is no property on IInvocationOperation to get this info directly (or at least I couldn't find it)

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you instead use

roslyn-analyzers/src/Utilities/Compiler/Extensions/IOperationExtensions.cs

Lines 23 to 28 in b7bb138

/// <summary>
/// Gets the receiver type for an invocation expression (i.e. type of 'A' in invocation 'A.B()')
/// If the invocation actually involves a conversion from A to some other type, say 'C', on which B is invoked,
/// then this method returns type A if <paramref name="beforeConversion"/> is true, and C if false.
/// </summary>
public static INamedTypeSymbol? GetReceiverType(this IInvocationOperation invocation, Compilation compilation, bool beforeConversion, CancellationToken cancellationToken)

Copy link
Member Author

@Youssef1313 Youssef1313 Jan 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It looks like GetReceiverType implementation is incorrect when beforeConversion is false.

firstArg.Type is always null. I'll use this extension and fix its implementation.


// If we're not in one of the known immutable types, quit
if (type is not null && type.GetBaseTypesAndThis().Any(immutableTypeSymbols.Contains))
{
context.ReportDiagnostic(invocation.CreateDiagnostic(DoNotIgnoreReturnValueDiagnosticRule, type.Name, methodName));
}
}
}
}
4 changes: 2 additions & 2 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.cs.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@
<note />
</trans-unit>
<trans-unit id="DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle">
<source>Do not ignore values returned by methods on immutable objects.</source>
<target state="translated">Neignorujte hodnoty vrácené metodami u neměnných objektů.</target>
<source>Do not ignore values returned by methods on immutable objects</source>
<target state="needs-review-translation">Neignorujte hodnoty vrácené metodami u neměnných objektů.</target>
<note />
</trans-unit>
<trans-unit id="OverrideGetFixAllProviderTitle">
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.de.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@
<note />
</trans-unit>
<trans-unit id="DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle">
<source>Do not ignore values returned by methods on immutable objects.</source>
<target state="translated">Keine von Methoden zu unveränderlichen Objekten zurückgegebenen Werte ignorieren</target>
<source>Do not ignore values returned by methods on immutable objects</source>
<target state="needs-review-translation">Keine von Methoden zu unveränderlichen Objekten zurückgegebenen Werte ignorieren</target>
<note />
</trans-unit>
<trans-unit id="OverrideGetFixAllProviderTitle">
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.es.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@
<note />
</trans-unit>
<trans-unit id="DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle">
<source>Do not ignore values returned by methods on immutable objects.</source>
<target state="translated">No omitir los valores devueltos por métodos en objetos inmutables.</target>
<source>Do not ignore values returned by methods on immutable objects</source>
<target state="needs-review-translation">No omitir los valores devueltos por métodos en objetos inmutables.</target>
<note />
</trans-unit>
<trans-unit id="OverrideGetFixAllProviderTitle">
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.fr.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@
<note />
</trans-unit>
<trans-unit id="DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle">
<source>Do not ignore values returned by methods on immutable objects.</source>
<target state="translated">N'ignorez pas les valeurs retournées par les méthodes sur les objets non modifiables.</target>
<source>Do not ignore values returned by methods on immutable objects</source>
<target state="needs-review-translation">N'ignorez pas les valeurs retournées par les méthodes sur les objets non modifiables.</target>
<note />
</trans-unit>
<trans-unit id="OverrideGetFixAllProviderTitle">
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.it.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@
<note />
</trans-unit>
<trans-unit id="DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle">
<source>Do not ignore values returned by methods on immutable objects.</source>
<target state="translated">Non ignorare i valori restituiti dai metodi su oggetti non modificabili.</target>
<source>Do not ignore values returned by methods on immutable objects</source>
<target state="needs-review-translation">Non ignorare i valori restituiti dai metodi su oggetti non modificabili.</target>
<note />
</trans-unit>
<trans-unit id="OverrideGetFixAllProviderTitle">
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ja.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@
<note />
</trans-unit>
<trans-unit id="DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle">
<source>Do not ignore values returned by methods on immutable objects.</source>
<target state="translated">不変オブジェクトのメソッドによって返される値を無視しないでください。</target>
<source>Do not ignore values returned by methods on immutable objects</source>
<target state="needs-review-translation">不変オブジェクトのメソッドによって返される値を無視しないでください。</target>
<note />
</trans-unit>
<trans-unit id="OverrideGetFixAllProviderTitle">
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ko.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@
<note />
</trans-unit>
<trans-unit id="DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle">
<source>Do not ignore values returned by methods on immutable objects.</source>
<target state="translated">변경할 수 없는 개체의 메서드에서 반환된 값을 무시하지 마세요.</target>
<source>Do not ignore values returned by methods on immutable objects</source>
<target state="needs-review-translation">변경할 수 없는 개체의 메서드에서 반환된 값을 무시하지 마세요.</target>
<note />
</trans-unit>
<trans-unit id="OverrideGetFixAllProviderTitle">
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pl.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@
<note />
</trans-unit>
<trans-unit id="DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle">
<source>Do not ignore values returned by methods on immutable objects.</source>
<target state="translated">Nie ignoruj wartości zwracanych przez metody dla niezmienialnych obiektów.</target>
<source>Do not ignore values returned by methods on immutable objects</source>
<target state="needs-review-translation">Nie ignoruj wartości zwracanych przez metody dla niezmienialnych obiektów.</target>
<note />
</trans-unit>
<trans-unit id="OverrideGetFixAllProviderTitle">
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.pt-BR.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@
<note />
</trans-unit>
<trans-unit id="DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle">
<source>Do not ignore values returned by methods on immutable objects.</source>
<target state="translated">Não ignore valores retornados por métodos em objetos imutáveis.</target>
<source>Do not ignore values returned by methods on immutable objects</source>
<target state="needs-review-translation">Não ignore valores retornados por métodos em objetos imutáveis.</target>
<note />
</trans-unit>
<trans-unit id="OverrideGetFixAllProviderTitle">
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.ru.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@
<note />
</trans-unit>
<trans-unit id="DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle">
<source>Do not ignore values returned by methods on immutable objects.</source>
<target state="translated">Не игнорируйте значения, возвращаемые методами для неизменяемых объектов.</target>
<source>Do not ignore values returned by methods on immutable objects</source>
<target state="needs-review-translation">Не игнорируйте значения, возвращаемые методами для неизменяемых объектов.</target>
<note />
</trans-unit>
<trans-unit id="OverrideGetFixAllProviderTitle">
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.CodeAnalysis.Analyzers/Core/xlf/CodeAnalysisDiagnosticsResources.tr.xlf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@
<note />
</trans-unit>
<trans-unit id="DoNotIgnoreReturnValueOnImmutableObjectMethodInvocationTitle">
<source>Do not ignore values returned by methods on immutable objects.</source>
<target state="translated">Sabit nesneler üzerinde yöntemler tarafından döndürülen değerleri yoksaymayın.</target>
<source>Do not ignore values returned by methods on immutable objects</source>
<target state="needs-review-translation">Sabit nesneler üzerinde yöntemler tarafından döndürülen değerleri yoksaymayın.</target>
<note />
</trans-unit>
<trans-unit id="OverrideGetFixAllProviderTitle">
Expand Down
Loading