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

RS0046: Avoid Opt suffix in nullable-enabled code #3352

Merged
merged 20 commits into from
Jun 6, 2020
Merged
Show file tree
Hide file tree
Changes from 16 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 @@ -2,5 +2,6 @@
Rule ID | Category | Severity | Notes
--------|----------|----------|-------
RS0038 | RoslynDiagnosticsMaintainability | Warning | PreferNullLiteral
RS0046 | RoslynDiagnosticsDesign | Warning | CSharpAvoidOptSuffixForNullableEnableCode
RS0100 | RoslynDiagnosticsMaintainability | Warning | CSharpWrapStatementsDiagnosticAnalyzer
RS0102 | RoslynDiagnosticsMaintainability | Warning | CSharpBracePlacementDiagnosticAnalyzer
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Analyzer.Utilities.Lightup;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Roslyn.Diagnostics.Analyzers;

namespace Roslyn.Diagnostics.CSharp.Analyzers
{
/// <summary>
/// RS0046: Avoid 'Opt' suffix for nullable enable code
mavasani marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class CSharpAvoidOptSuffixForNullableEnableCode : DiagnosticAnalyzer
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
{
internal const string OptSuffix = "Opt";

private static readonly LocalizableString s_localizableTitle = new LocalizableResourceString(nameof(RoslynDiagnosticsAnalyzersResources.AvoidOptSuffixForNullableEnableCodeRuleIdTitle), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));
private static readonly LocalizableString s_localizableMessage = new LocalizableResourceString(nameof(RoslynDiagnosticsAnalyzersResources.AvoidOptSuffixForNullableEnableCodeRuleIdMessage), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));
private static readonly LocalizableString s_localizableDescription = new LocalizableResourceString(nameof(RoslynDiagnosticsAnalyzersResources.AvoidOptSuffixForNullableEnableCodeRuleIdDescription), RoslynDiagnosticsAnalyzersResources.ResourceManager, typeof(RoslynDiagnosticsAnalyzersResources));
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

internal static DiagnosticDescriptor Rule = new DiagnosticDescriptor(
RoslynDiagnosticIds.AvoidOptSuffixForNullableEnableCodeRuleId,
s_localizableTitle,
s_localizableMessage,
DiagnosticCategory.RoslynDiagnosticsDesign,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description: s_localizableDescription,
helpLinkUri: null,
customTags: WellKnownDiagnosticTags.Telemetry);

public sealed override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

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

context.RegisterSyntaxNodeAction(context =>
{
var parameter = (ParameterSyntax)context.Node;
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
ReportOnInvalidIdentifier(parameter.Identifier, context.SemanticModel, context.ReportDiagnostic);
}, SyntaxKind.Parameter);
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

context.RegisterSyntaxNodeAction(context =>
{
var variableDeclarator = (VariableDeclaratorSyntax)context.Node;
ReportOnInvalidIdentifier(variableDeclarator.Identifier, context.SemanticModel, context.ReportDiagnostic);
}, SyntaxKind.VariableDeclarator);
}

private static void ReportOnInvalidIdentifier(SyntaxToken identifier, SemanticModel semanticModel, Action<Diagnostic> reportAction)
{
if (identifier.Text.EndsWith(OptSuffix, StringComparison.Ordinal) &&
semanticModel.GetNullableContext(identifier.SpanStart).AnnotationsEnabled())
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
{
reportAction(identifier.CreateDiagnostic(Rule));
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Immutable;
using System.Composition;
using System.Threading;
using System.Threading.Tasks;
using Analyzer.Utilities;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Rename;
using Roslyn.Diagnostics.Analyzers;

namespace Roslyn.Diagnostics.CSharp.Analyzers
{
[ExportCodeFixProvider(LanguageNames.CSharp)]
[Shared]
public sealed class CSharpAvoidOptSuffixForNullableEnableCodeCodeFixProvider : CodeFixProvider
{
public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(CSharpAvoidOptSuffixForNullableEnableCode.Rule.Id);

public override FixAllProvider GetFixAllProvider()
=> WellKnownFixAllProviders.BatchFixer;

public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var title = RoslynDiagnosticsAnalyzersResources.AvoidOptSuffixForNullableEnableCodeRuleIdTitleCodeFixTitle;
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

foreach (var diagnostic in context.Diagnostics)
{
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
if (root == null)
{
continue;
}

Evangelink marked this conversation as resolved.
Show resolved Hide resolved
var variable = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);
if (variable == null)
{
continue;
}

var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
var variableSymbol = semanticModel.GetDeclaredSymbol(variable, context.CancellationToken);
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 try to write some test with error code where this can return null? Honestly, I am not sure but worth trying out few code snippets.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have tried a couple of tests and they all return a non-null symbol. If you have any suggestion I am happy to add a test. @sharwell Any idea?

if (variableSymbol == null)
{
continue;
}

var newName = variableSymbol.Name.Substring(0, variableSymbol.Name.Length - CSharpAvoidOptSuffixForNullableEnableCode.OptSuffix.Length);
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

// There is no symbol matching the new name so we can register the codefix
if (semanticModel.LookupSymbols(diagnostic.Location.SourceSpan.Start, variableSymbol.ContainingType, newName).IsEmpty)
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
{
context.RegisterCodeFix(
new MyCodeAction(
title,
cancellationToken => RemoveOptSuffixOnVariableAsync(context.Document, variableSymbol, newName, cancellationToken),
equivalenceKey: title),
diagnostic);
}
}
}

private static async Task<Document> RemoveOptSuffixOnVariableAsync(Document document, ISymbol variableSymbol, string newName, CancellationToken cancellationToken)
{
var newSolution = await Renamer.RenameSymbolAsync(document.Project.Solution, variableSymbol, newName, document.Project.Solution.Options, cancellationToken)
.ConfigureAwait(false);

return newSolution.GetDocument(document.Id)!;
}
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

// Needed for Telemetry (https://github.com/dotnet/roslyn/issues/4919)
private class MyCodeAction : DocumentChangeAction
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
{
public MyCodeAction(string title, Func<CancellationToken, Task<Document>> createChangedDocument, string equivalenceKey)
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
: base(title, createChangedDocument, equivalenceKey)
{
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ internal static class RoslynDiagnosticIds
public const string DoNotCallGetTestAccessorRuleId = "RS0043";
public const string CreateTestAccessorRuleId = "RS0044";
public const string ExposeMemberForTestingRuleId = "RS0045";
public const string AvoidOptSuffixForNullableEnableCodeRuleId = "RS0046";
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

public const string WrapStatementsRuleId = "RS0100";
public const string BlankLinesRuleId = "RS0101";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,15 @@
<data name="ExposeMemberForTestingTitle" xml:space="preserve">
<value>Expose member for testing</value>
</data>
<data name="AvoidOptSuffixForNullableEnableCodeRuleIdDescription" xml:space="preserve">
<value>Avoid the 'Opt' suffix in a nullable-enabled code.</value>
</data>
<data name="AvoidOptSuffixForNullableEnableCodeRuleIdMessage" xml:space="preserve">
<value>Avoid the 'Opt' suffix in a nullable-enabled code</value>
</data>
<data name="AvoidOptSuffixForNullableEnableCodeRuleIdTitle" xml:space="preserve">
<value>Avoid the 'Opt' suffix</value>
</data>
<data name="BracePlacementMessage" xml:space="preserve">
<value>Braces must not have blank lines between them</value>
</data>
Expand All @@ -357,4 +366,7 @@
<data name="Place_statement_on_following_line" xml:space="preserve">
<value>Place statement on following line</value>
</data>
<data name="AvoidOptSuffixForNullableEnableCodeRuleIdTitleCodeFixTitle" xml:space="preserve">
<value>Remove the 'Opt' suffix</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,26 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="cs" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdDescription">
<source>Avoid the 'Opt' suffix in a nullable-enabled code.</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code.</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdMessage">
<source>Avoid the 'Opt' suffix in a nullable-enabled code</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitle">
<source>Avoid the 'Opt' suffix</source>
<target state="new">Avoid the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitleCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="new">Remove the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="new">Avoid multiple blank lines</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,26 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="de" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdDescription">
<source>Avoid the 'Opt' suffix in a nullable-enabled code.</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code.</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdMessage">
<source>Avoid the 'Opt' suffix in a nullable-enabled code</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitle">
<source>Avoid the 'Opt' suffix</source>
<target state="new">Avoid the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitleCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="new">Remove the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="new">Avoid multiple blank lines</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,26 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="es" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdDescription">
<source>Avoid the 'Opt' suffix in a nullable-enabled code.</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code.</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdMessage">
<source>Avoid the 'Opt' suffix in a nullable-enabled code</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitle">
<source>Avoid the 'Opt' suffix</source>
<target state="new">Avoid the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitleCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="new">Remove the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="new">Avoid multiple blank lines</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,26 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="fr" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdDescription">
<source>Avoid the 'Opt' suffix in a nullable-enabled code.</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code.</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdMessage">
<source>Avoid the 'Opt' suffix in a nullable-enabled code</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitle">
<source>Avoid the 'Opt' suffix</source>
<target state="new">Avoid the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitleCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="new">Remove the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="new">Avoid multiple blank lines</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,26 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="it" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdDescription">
<source>Avoid the 'Opt' suffix in a nullable-enabled code.</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code.</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdMessage">
<source>Avoid the 'Opt' suffix in a nullable-enabled code</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitle">
<source>Avoid the 'Opt' suffix</source>
<target state="new">Avoid the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitleCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="new">Remove the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="new">Avoid multiple blank lines</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,26 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="ja" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdDescription">
<source>Avoid the 'Opt' suffix in a nullable-enabled code.</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code.</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdMessage">
<source>Avoid the 'Opt' suffix in a nullable-enabled code</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitle">
<source>Avoid the 'Opt' suffix</source>
<target state="new">Avoid the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitleCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="new">Remove the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="new">Avoid multiple blank lines</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,26 @@
<xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.2" xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 xliff-core-1.2-transitional.xsd">
<file datatype="xml" source-language="en" target-language="ko" original="../RoslynDiagnosticsAnalyzersResources.resx">
<body>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdDescription">
<source>Avoid the 'Opt' suffix in a nullable-enabled code.</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code.</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdMessage">
<source>Avoid the 'Opt' suffix in a nullable-enabled code</source>
<target state="new">Avoid the 'Opt' suffix in a nullable-enabled code</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitle">
<source>Avoid the 'Opt' suffix</source>
<target state="new">Avoid the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="AvoidOptSuffixForNullableEnableCodeRuleIdTitleCodeFixTitle">
<source>Remove the 'Opt' suffix</source>
<target state="new">Remove the 'Opt' suffix</target>
<note />
</trans-unit>
<trans-unit id="BlankLinesMessage">
<source>Avoid multiple blank lines</source>
<target state="new">Avoid multiple blank lines</target>
Expand Down
Loading