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

Jump to bottom

Make UtilityAnalyzerBase stateless by removing properties #6902

Merged
merged 6 commits into from
May 19, 2023
Merged

Make UtilityAnalyzerBase stateless by removing properties #6902

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
10c87f3
Make UtilityAnalyzerBase stateless by removing properties.
martin-strecker-sonarsource Mar 10, 2023
0f1adae
Formattings
martin-strecker-sonarsource Mar 10, 2023
a1467d3
Simplify TestUtilityAnalyzer
martin-strecker-sonarsource Mar 10, 2023
6894ca5
Splt line
martin-strecker-sonarsource Mar 14, 2023
d9589fd
rename parameter to parameters and small re-arrange of class members
mary-georgiou-sonarsource May 17, 2023
dbdcf98
fix code smell
mary-georgiou-sonarsource May 17, 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: 3 additions & 3 deletions analyzers/src/SonarAnalyzer.Common/Rules/Utilities/AnalysisWarningAnalyzerBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ protected AnalysisWarningAnalyzerBase() : base(DiagnosticId, Title) { }
protected sealed override void Initialize(SonarAnalysisContext context) =>
context.RegisterCompilationStartAction(c =>
{
ReadParameters(c);
if (IsAnalyzerEnabled && !RoslynHelper.IsRoslynCfgSupported(MinimalSupportedRoslynVersion)) // MsBuild 15 is bound with Roslyn 2.x, where Roslyn CFG is not available.
var parameter = ReadParameters(c);
if (parameter.IsAnalyzerEnabled && !RoslynHelper.IsRoslynCfgSupported(MinimalSupportedRoslynVersion)) // MsBuild 15 is bound with Roslyn 2.x, where Roslyn CFG is not available.
{
// This can be removed after we bump Microsoft.CodeAnalysis references to 3.0 or higher.
var path = Path.GetFullPath(Path.Combine(OutPath, "../../AnalysisWarnings.MsBuild.json"));
var path = Path.GetFullPath(Path.Combine(parameter.OutPath, "../../AnalysisWarnings.MsBuild.json"));
lock (FileWriteLock)
{
if (!File.Exists(path))
Expand Down
6 changes: 4 additions & 2 deletions analyzers/src/SonarAnalyzer.Common/Rules/Utilities/CopyPasteTokenAnalyzerBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@ public abstract class CopyPasteTokenAnalyzerBase<TSyntaxKind> : UtilityAnalyzerB
protected abstract string GetCpdValue(SyntaxToken token);
protected abstract bool IsUsingDirective(SyntaxNode node);

protected sealed override bool AnalyzeTestProjects => false;
protected override UtilityAnalyzerParameters ReadParameters(SonarCompilationStartAnalysisContext context) =>
base.ReadParameters(context) with { AnalyzeTestProjects = false };

protected sealed override bool AnalyzeUnchangedFiles => true;
protected sealed override string FileName => "token-cpd.pb";

protected CopyPasteTokenAnalyzerBase() : base(DiagnosticId, Title) { }

protected sealed override CopyPasteTokenInfo CreateMessage(SyntaxTree syntaxTree, SemanticModel semanticModel)
protected sealed override CopyPasteTokenInfo CreateMessage(UtilityAnalyzerParameters parameters, SyntaxTree syntaxTree, SemanticModel semanticModel)
{
var cpdTokenInfo = new CopyPasteTokenInfo { FilePath = syntaxTree.FilePath };
foreach (var token in syntaxTree.GetRoot().DescendantTokens(n => !IsUsingDirective(n)))
Expand Down
7 changes: 4 additions & 3 deletions analyzers/src/SonarAnalyzer.Common/Rules/Utilities/FileMetadataAnalyzerBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,13 @@ public abstract class FileMetadataAnalyzerBase<TSyntaxKind> : UtilityAnalyzerBas
private const string Title = "File metadata generator";

protected sealed override string FileName => "file-metadata.pb";
protected override bool AnalyzeGeneratedCode => true;
protected override UtilityAnalyzerParameters ReadParameters(SonarCompilationStartAnalysisContext context) =>
base.ReadParameters(context) with { AnalyzeGeneratedCode = true };

protected FileMetadataAnalyzerBase() : base(DiagnosticId, Title) { }

protected sealed override FileMetadataInfo CreateMessage(SyntaxTree syntaxTree, SemanticModel semanticModel) =>
new FileMetadataInfo
protected sealed override FileMetadataInfo CreateMessage(UtilityAnalyzerParameters parameters, SyntaxTree syntaxTree, SemanticModel semanticModel) =>
new()
{
FilePath = syntaxTree.FilePath,
IsGenerated = Language.GeneratedCodeRecognizer.IsGenerated(syntaxTree),
Expand Down
5 changes: 3 additions & 2 deletions analyzers/src/SonarAnalyzer.Common/Rules/Utilities/LogAnalyzerBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ public abstract class LogAnalyzerBase<TSyntaxKind> : UtilityAnalyzerBase<TSyntax
protected abstract string LanguageVersion(Compilation compilation);

protected sealed override string FileName => "log.pb";
protected override bool AnalyzeGeneratedCode => true;
protected override UtilityAnalyzerParameters ReadParameters(SonarCompilationStartAnalysisContext context) =>
base.ReadParameters(context) with { AnalyzeGeneratedCode = true };

protected LogAnalyzerBase() : base(DiagnosticId, Title) { }

Expand All @@ -43,7 +44,7 @@ protected sealed override IEnumerable<LogInfo> CreateAnalysisMessages(SonarCompi
new LogInfo { Severity = LogSeverity.Info, Text = "Concurrent execution: " + (IsConcurrentExecutionEnabled() ? "enabled" : "disabled") }
};

protected sealed override LogInfo CreateMessage(SyntaxTree syntaxTree, SemanticModel semanticModel) =>
protected sealed override LogInfo CreateMessage(UtilityAnalyzerParameters parameters, SyntaxTree syntaxTree, SemanticModel semanticModel) =>
syntaxTree.IsGenerated(Language.GeneratedCodeRecognizer, semanticModel.Compilation)
? new LogInfo { Severity = LogSeverity.Debug, Text = $"File '{syntaxTree.FilePath}' was recognized as generated" }
: null;
Expand Down
7 changes: 4 additions & 3 deletions analyzers/src/SonarAnalyzer.Common/Rules/Utilities/MetricsAnalyzerBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@ public abstract class MetricsAnalyzerBase<TSyntaxKind> : UtilityAnalyzerBase<TSy
protected abstract MetricsBase GetMetrics(SyntaxTree syntaxTree, SemanticModel semanticModel);

protected sealed override string FileName => "metrics.pb";
protected sealed override bool AnalyzeTestProjects => false;
protected override UtilityAnalyzerParameters ReadParameters(SonarCompilationStartAnalysisContext context) =>
base.ReadParameters(context) with { AnalyzeTestProjects = false };

protected MetricsAnalyzerBase() : base(DiagnosticId, Title) { }

protected sealed override MetricsInfo CreateMessage(SyntaxTree syntaxTree, SemanticModel semanticModel)
protected sealed override MetricsInfo CreateMessage(UtilityAnalyzerParameters parameters, SyntaxTree syntaxTree, SemanticModel semanticModel)
{
var metrics = GetMetrics(syntaxTree, semanticModel);
var complexity = metrics.Complexity;
Expand All @@ -49,7 +50,7 @@ protected sealed override MetricsInfo CreateMessage(SyntaxTree syntaxTree, Seman
CognitiveComplexity = metrics.CognitiveComplexity,
};

var comments = metrics.GetComments(IgnoreHeaderComments);
var comments = metrics.GetComments(parameters.IgnoreHeaderComments);
metricsInfo.NoSonarComment.AddRange(comments.NoSonar);
metricsInfo.NonBlankComment.AddRange(comments.NonBlank);
metricsInfo.CodeLine.AddRange(metrics.CodeLines);
Expand Down
6 changes: 3 additions & 3 deletions analyzers/src/SonarAnalyzer.Common/Rules/Utilities/SymbolReferenceAnalyzerBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public abstract class SymbolReferenceAnalyzerBase<TSyntaxKind> : UtilityAnalyzer

protected SymbolReferenceAnalyzerBase() : base(DiagnosticId, Title) { }

protected sealed override SymbolReferenceInfo CreateMessage(SyntaxTree syntaxTree, SemanticModel semanticModel)
protected sealed override SymbolReferenceInfo CreateMessage(UtilityAnalyzerParameters parameters, SyntaxTree syntaxTree, SemanticModel semanticModel)
{
var symbolReferenceInfo = new SymbolReferenceInfo { FilePath = syntaxTree.FilePath };
var references = GetReferences(syntaxTree.GetRoot(), semanticModel);
Expand All @@ -56,8 +56,8 @@ protected sealed override SymbolReferenceInfo CreateMessage(SyntaxTree syntaxTre
return symbolReferenceInfo;
}

protected override bool ShouldGenerateMetrics(SyntaxTree tree) =>
base.ShouldGenerateMetrics(tree)
protected override bool ShouldGenerateMetrics(UtilityAnalyzerParameters parameters, SyntaxTree tree) =>
base.ShouldGenerateMetrics(parameters, tree)
&& !HasTooManyTokens(tree);

private Dictionary<ISymbol, List<ReferenceInfo>> GetReferences(SyntaxNode root, SemanticModel model)
Expand Down
2 changes: 1 addition & 1 deletion analyzers/src/SonarAnalyzer.Common/Rules/Utilities/TokenTypeAnalyzerBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ protected TokenTypeAnalyzerBase() : base(DiagnosticId, Title) { }
protected abstract TokenClassifierBase GetTokenClassifier(SemanticModel semanticModel, bool skipIdentifierTokens);
protected abstract TriviaClassifierBase GetTriviaClassifier();

protected sealed override TokenTypeInfo CreateMessage(SyntaxTree syntaxTree, SemanticModel semanticModel)
protected sealed override TokenTypeInfo CreateMessage(UtilityAnalyzerParameters parameters, SyntaxTree syntaxTree, SemanticModel semanticModel)
{
var tokens = syntaxTree.GetRoot().DescendantTokens();
var identifierTokenKind = Language.SyntaxKind.IdentifierToken; // Performance optimization
Expand Down
59 changes: 32 additions & 27 deletions analyzers/src/SonarAnalyzer.Common/Rules/Utilities/UtilityAnalyzerBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,21 @@

namespace SonarAnalyzer.Rules
{
public readonly record struct UtilityAnalyzerParameters(bool IsAnalyzerEnabled, bool IgnoreHeaderComments, bool AnalyzeGeneratedCode, bool AnalyzeTestProjects, string OutPath, bool IsTestProject)
{
public static readonly UtilityAnalyzerParameters Default =
new(IsAnalyzerEnabled: false, IgnoreHeaderComments: false, AnalyzeGeneratedCode: false, AnalyzeTestProjects: true, OutPath: null, IsTestProject: false);
}

public abstract class UtilityAnalyzerBase : SonarDiagnosticAnalyzer
{
protected static readonly ISet<string> FileExtensionWhitelist = new HashSet<string> { ".cs", ".csx", ".vb" };
private readonly DiagnosticDescriptor rule;

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(rule);
protected bool IsAnalyzerEnabled { get; set; }
protected bool IgnoreHeaderComments { get; set; }
protected virtual bool AnalyzeGeneratedCode { get; set; }
protected virtual bool AnalyzeTestProjects => true;
protected string OutPath { get; set; }
protected bool IsTestProject { get; set; }
protected override bool EnableConcurrentExecution => false;
Comment on lines -33 to 37
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like with the OutPath property, the mutable state is the root cause that prevents ConcurrentExecution (to be fair: ConcurrentExecution would also work without this change, but it is tough to see why that is the case. With this change, it is evident that it works, and it is no longer possible to break it).

The PR moves the state into its own type, which is now captured in RegisterCompilationStartAction for the compilation. The new Parameters struct is readonly now and passed to all methods that need it to make decisions.


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

protected UtilityAnalyzerBase(string diagnosticId, string title) =>
rule = DiagnosticDescriptorFactory.CreateUtility(diagnosticId, title);

Expand All @@ -50,7 +51,7 @@ internal static TextRange GetTextRange(FileLinePositionSpan lineSpan) =>
EndOffset = lineSpan.EndLinePosition.Character
};

protected void ReadParameters(SonarCompilationStartAnalysisContext context)
protected virtual UtilityAnalyzerParameters ReadParameters(SonarCompilationStartAnalysisContext context)
{
var outPath = context.ProjectConfiguration().OutPath;
// For backward compatibility with S4MSB <= 5.0
Expand All @@ -60,13 +61,17 @@ protected void ReadParameters(SonarCompilationStartAnalysisContext context)
}
if (context.Options.SonarLintXml() != null && !string.IsNullOrEmpty(outPath))
{
var language = context.Compilation.Language;
var sonarLintXml = context.SonarLintXml();
IgnoreHeaderComments = sonarLintXml.IgnoreHeaderComments(context.Compilation.Language);
AnalyzeGeneratedCode = sonarLintXml.AnalyzeGeneratedCode(context.Compilation.Language);
OutPath = Path.Combine(outPath, context.Compilation.Language == LanguageNames.CSharp ? "output-cs" : "output-vbnet");
IsAnalyzerEnabled = true;
IsTestProject = context.IsTestProject();
return new UtilityAnalyzerParameters(
IsAnalyzerEnabled: true,
IgnoreHeaderComments: sonarLintXml.IgnoreHeaderComments(language),
AnalyzeGeneratedCode: sonarLintXml.AnalyzeGeneratedCode(language),
AnalyzeTestProjects: true,
OutPath: Path.Combine(outPath, language == LanguageNames.CSharp ? "output-cs" : "output-vbnet"),
IsTestProject: context.IsTestProject());
}
return UtilityAnalyzerParameters.Default;
}
}

Expand All @@ -78,29 +83,29 @@ public abstract class UtilityAnalyzerBase<TSyntaxKind, TMessage> : UtilityAnalyz

protected abstract ILanguageFacade<TSyntaxKind> Language { get; }
protected abstract string FileName { get; }
protected abstract TMessage CreateMessage(SyntaxTree syntaxTree, SemanticModel semanticModel);

protected virtual bool AnalyzeUnchangedFiles => false;

protected abstract TMessage CreateMessage(UtilityAnalyzerParameters parameters, SyntaxTree syntaxTree, SemanticModel semanticModel);

protected virtual IEnumerable<TMessage> CreateAnalysisMessages(SonarCompilationReportingContext c) => Enumerable.Empty<TMessage>();

protected UtilityAnalyzerBase(string diagnosticId, string title) : base(diagnosticId, title) { }

protected sealed override void Initialize(SonarAnalysisContext context) =>
context.RegisterCompilationStartAction(startContext =>
{
ReadParameters(startContext);
if (!IsAnalyzerEnabled)
var parameters = ReadParameters(startContext);
if (!parameters.IsAnalyzerEnabled)
{
return;
}

var treeMessages = new List<TMessage>();
startContext.RegisterSemanticModelAction(modelContext =>
{
if (ShouldGenerateMetrics(modelContext))
if (ShouldGenerateMetrics(parameters, modelContext))
{
treeMessages.Add(CreateMessage(modelContext.Tree, modelContext.SemanticModel));
treeMessages.Add(CreateMessage(parameters, modelContext.Tree, modelContext.SemanticModel));
}
});

Expand All @@ -112,8 +117,8 @@ protected sealed override void Initialize(SonarAnalysisContext context) =>
.ToArray();
lock (FileWriteLock)
{
Directory.CreateDirectory(OutPath);
using var stream = File.Create(Path.Combine(OutPath, FileName));
Directory.CreateDirectory(parameters.OutPath);
using var stream = File.Create(Path.Combine(parameters.OutPath, FileName));
foreach (var message in allMessages)
{
message.WriteDelimitedTo(stream);
Expand All @@ -122,14 +127,14 @@ protected sealed override void Initialize(SonarAnalysisContext context) =>
});
});

protected virtual bool ShouldGenerateMetrics(SyntaxTree tree) =>
protected virtual bool ShouldGenerateMetrics(UtilityAnalyzerParameters parameters, SyntaxTree tree) =>
// The results of Metrics and CopyPasteToken analyzers are not needed for Test projects yet the plugin side expects the protobuf files, so we create empty ones.
(AnalyzeTestProjects || !IsTestProject)
(parameters.AnalyzeTestProjects || !parameters.IsTestProject)
&& FileExtensionWhitelist.Contains(Path.GetExtension(tree.FilePath))
&& (AnalyzeGeneratedCode || !Language.GeneratedCodeRecognizer.IsGenerated(tree));
&& (parameters.AnalyzeGeneratedCode || !Language.GeneratedCodeRecognizer.IsGenerated(tree));

private bool ShouldGenerateMetrics(SonarSematicModelReportingContext context) =>
private bool ShouldGenerateMetrics(UtilityAnalyzerParameters parameters, SonarSematicModelReportingContext context) =>
(AnalyzeUnchangedFiles || !context.IsUnchanged(context.Tree))
&& ShouldGenerateMetrics(context.Tree);
&& ShouldGenerateMetrics(parameters, context.Tree);
}
}
19 changes: 15 additions & 4 deletions analyzers/tests/SonarAnalyzer.UnitTest/Rules/Utilities/AnalysisWarningAnalyzerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
*/

using System.IO;
using SonarAnalyzer.AnalysisContext;
using SonarAnalyzer.CFG.Helpers;
using SonarAnalyzer.Common;
using SonarAnalyzer.Rules;
Expand Down Expand Up @@ -88,25 +89,35 @@ private string ExecuteAnalyzer(string languageName, bool isAnalyzerEnabled, int

private sealed class TestAnalysisWarningAnalyzer_CS : CS.AnalysisWarningAnalyzer
{
private readonly bool isAnalyzerEnabled;
private readonly string outPath;
protected override int MinimalSupportedRoslynVersion { get; }

public TestAnalysisWarningAnalyzer_CS(bool isAnalyzerEnabled, int minimalSupportedRoslynVersion, string outPath)
{
IsAnalyzerEnabled = isAnalyzerEnabled;
this.isAnalyzerEnabled = isAnalyzerEnabled;
MinimalSupportedRoslynVersion = minimalSupportedRoslynVersion;
OutPath = outPath;
this.outPath = outPath;
}

protected override UtilityAnalyzerParameters ReadParameters(SonarCompilationStartAnalysisContext context) =>
base.ReadParameters(context) with { IsAnalyzerEnabled = isAnalyzerEnabled, OutPath = outPath };
}

private sealed class TestAnalysisWarningAnalyzer_VB : VB.AnalysisWarningAnalyzer
{
private readonly bool isAnalyzerEnabled;
private readonly string outPath;
protected override int MinimalSupportedRoslynVersion { get; }

public TestAnalysisWarningAnalyzer_VB(bool isAnalyzerEnabled, int minimalSupportedRoslynVersion, string outPath)
{
IsAnalyzerEnabled = isAnalyzerEnabled;
this.isAnalyzerEnabled = isAnalyzerEnabled;
MinimalSupportedRoslynVersion = minimalSupportedRoslynVersion;
OutPath = outPath;
this.outPath = outPath;
}

protected override UtilityAnalyzerParameters ReadParameters(SonarCompilationStartAnalysisContext context) =>
base.ReadParameters(context) with { IsAnalyzerEnabled = isAnalyzerEnabled, OutPath = outPath };
}
}
23 changes: 17 additions & 6 deletions analyzers/tests/SonarAnalyzer.UnitTest/Rules/Utilities/CopyPasteTokenAnalyzerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
*/

using System.IO;
using SonarAnalyzer.AnalysisContext;
using SonarAnalyzer.Common;
using SonarAnalyzer.Protobuf;
using SonarAnalyzer.Rules;
Expand Down Expand Up @@ -141,22 +142,32 @@ private VerifierBuilder CreateBuilder(ProjectType projectType, string fileName)
// We need to set protected properties and this class exists just to enable the analyzer without bothering with additional files with parameters
private sealed class TestCopyPasteTokenAnalyzer_CS : CS.CopyPasteTokenAnalyzer
{
private readonly string outPath;
private readonly bool isTestProject;

public TestCopyPasteTokenAnalyzer_CS(string outPath, bool isTestProject)
{
IsAnalyzerEnabled = true;
OutPath = outPath;
IsTestProject = isTestProject;
this.outPath = outPath;
this.isTestProject = isTestProject;
}

protected override UtilityAnalyzerParameters ReadParameters(SonarCompilationStartAnalysisContext context) =>
base.ReadParameters(context) with { IsAnalyzerEnabled = true, OutPath = outPath, IsTestProject = isTestProject };
}

private sealed class TestCopyPasteTokenAnalyzer_VB : VB.CopyPasteTokenAnalyzer
{
private readonly string outPath;
private readonly bool isTestProject;

public TestCopyPasteTokenAnalyzer_VB(string outPath, bool isTestProject)
{
IsAnalyzerEnabled = true;
OutPath = outPath;
IsTestProject = isTestProject;
this.outPath = outPath;
this.isTestProject = isTestProject;
}

protected override UtilityAnalyzerParameters ReadParameters(SonarCompilationStartAnalysisContext context) =>
base.ReadParameters(context) with { IsAnalyzerEnabled = true, OutPath = outPath, IsTestProject = isTestProject };
}
}
}
Loading