From 681e884aa8adba45f3afd6456a0af8149c2e5082 Mon Sep 17 00:00:00 2001 From: Jb Evain Date: Wed, 13 May 2026 14:32:01 -0700 Subject: [PATCH] Add conditional compilation symbol typo analyzer Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/UNT0043.md | 35 +++ doc/index.md | 1 + .../ConditionalCompilationSymbolTypoTests.cs | 205 ++++++++++++++ .../AnalyzerVerificationContext.cs | 24 +- .../Infrastructure/DiagnosticVerifier.cs | 5 +- .../ConditionalCompilationSymbolTypo.cs | 257 ++++++++++++++++++ .../Resources/Strings.Designer.cs | 38 ++- .../Resources/Strings.resx | 14 + 8 files changed, 572 insertions(+), 7 deletions(-) create mode 100644 doc/UNT0043.md create mode 100644 src/Microsoft.Unity.Analyzers.Tests/ConditionalCompilationSymbolTypoTests.cs create mode 100644 src/Microsoft.Unity.Analyzers/ConditionalCompilationSymbolTypo.cs diff --git a/doc/UNT0043.md b/doc/UNT0043.md new file mode 100644 index 00000000..8ab24077 --- /dev/null +++ b/doc/UNT0043.md @@ -0,0 +1,35 @@ +# UNT0043 Possible typo in conditional compilation symbol + +Mistyped conditional compilation symbols in `#if` and `#elif` directives are treated as undefined, so the affected branch can be compiled or skipped unexpectedly without a compiler error. + +## Examples of patterns that are flagged by this analyzer + +```csharp +#if UNITY_EDITOR || UNITY_STANDALONE_WIN || UNITY_STANDALONE_LINUX || UNITTY_STANDALONE_OSX + InitializeServiceLocator(); +#endif +``` + +If the project defines `UNITY_STANDALONE_OSX`, `UNITTY_STANDALONE_OSX` is reported as very close to that project-level preprocessor symbol. The analyzer gets this symbol set from Roslyn's parse options for the project. + +The same applies to custom project symbols: + +```csharp +#if FEATURE_RELEAS + EnableReleaseFeature(); +#endif +``` + +If the project defines `FEATURE_RELEASE`, this is reported as a likely typo. + +## Solution + +Use the intended symbol name: + +```csharp +#if UNITY_EDITOR || UNITY_STANDALONE_WIN || UNITY_STANDALONE_LINUX || UNITY_STANDALONE_OSX + InitializeServiceLocator(); +#endif +``` + +A code fix is offered for this diagnostic to replace the mistyped symbol with the suggested project-level symbol. diff --git a/doc/index.md b/doc/index.md index a40625d2..92bf50bf 100644 --- a/doc/index.md +++ b/doc/index.md @@ -44,6 +44,7 @@ ID | Title | Category [UNT0040](UNT0040.md) | `GameObject.isStatic` is editor-only | Correctness [UNT0041](UNT0041.md) | Use `Animator.StringToHash` for repeated `Animator` method calls | Performance [UNT0042](UNT0042.md) | `Mesh` array property accessed in loop | Performance +[UNT0043](UNT0043.md) | Possible typo in conditional compilation symbol | Correctness # Diagnostic Suppressors diff --git a/src/Microsoft.Unity.Analyzers.Tests/ConditionalCompilationSymbolTypoTests.cs b/src/Microsoft.Unity.Analyzers.Tests/ConditionalCompilationSymbolTypoTests.cs new file mode 100644 index 00000000..3cacf0e0 --- /dev/null +++ b/src/Microsoft.Unity.Analyzers.Tests/ConditionalCompilationSymbolTypoTests.cs @@ -0,0 +1,205 @@ +/*-------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *-------------------------------------------------------------------------------------------*/ + +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.Unity.Analyzers.Tests; + +public class ConditionalCompilationSymbolTypoTests : BaseCodeFixVerifierTest +{ + [Fact] + public async Task UnityPlatformSymbolTypo() + { + const string test = @" +class Test +{ + void Method() + { +#if UNITTY_STANDALONE_OSX + return; +#endif + } +} +"; + + var diagnostic = ExpectDiagnostic() + .WithLocation(6, 5) + .WithArguments("UNITTY_STANDALONE_OSX", "UNITY_STANDALONE_OSX"); + + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols("UNITY_STANDALONE_OSX"); + + await VerifyCSharpDiagnosticAsync(context, test, diagnostic); + + const string fixedTest = @" +class Test +{ + void Method() + { +#if UNITY_STANDALONE_OSX + return; +#endif + } +} +"; + + await VerifyCSharpFixAsync(context, test, fixedTest); + } + + [Fact] + public async Task CompoundConditionTypoTrivia() + { + const string test = @" +class Test +{ + void Method() + { +#if UNITY_EDITOR || UNITY_STANDALONE_WIN || UNITY_STANDALONE_LINUX || !UNITTY_STANDALONE_OSX + return; +#endif + } +} +"; + + var diagnostic = ExpectDiagnostic() + .WithLocation(6, 72) + .WithArguments("UNITTY_STANDALONE_OSX", "UNITY_STANDALONE_OSX"); + + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols("UNITY_EDITOR", "UNITY_STANDALONE_WIN", "UNITY_STANDALONE_LINUX", "UNITY_STANDALONE_OSX"); + + await VerifyCSharpDiagnosticAsync(context, test, diagnostic); + + const string fixedTest = @" +class Test +{ + void Method() + { +#if UNITY_EDITOR || UNITY_STANDALONE_WIN || UNITY_STANDALONE_LINUX || !UNITY_STANDALONE_OSX + return; +#endif + } +} +"; + + await VerifyCSharpFixAsync(context, test, fixedTest); + } + + [Fact] + public async Task ElifDirectiveTypo() + { + const string test = @" +class Test +{ + void Method() + { +#if UNITY_EDITOR + return; +#elif UNITTY_STANDALONE_OSX + return; +#endif + } +} +"; + + var diagnostic = ExpectDiagnostic() + .WithLocation(8, 7) + .WithArguments("UNITTY_STANDALONE_OSX", "UNITY_STANDALONE_OSX"); + + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols("UNITY_EDITOR", "UNITY_STANDALONE_OSX"); + + await VerifyCSharpDiagnosticAsync(context, test, diagnostic); + } + + [Fact] + public async Task ProjectSymbolTypo() + { + const string test = @" +class Test +{ + void Method() + { +#if FEATURE_RELEAS + return; +#endif + } +} +"; + + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols("FEATURE_RELEASE"); + + var diagnostic = ExpectDiagnostic() + .WithLocation(6, 5) + .WithArguments("FEATURE_RELEAS", "FEATURE_RELEASE"); + + await VerifyCSharpDiagnosticAsync(context, test, diagnostic); + } + + [Fact] + public async Task DefinedProjectSymbol() + { + const string test = @" +class Test +{ + void Method() + { +#if FEATURE_RELEASE + return; +#endif + } +} +"; + + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols("FEATURE_RELEASE"); + + await VerifyCSharpDiagnosticAsync(context, test); + } + + [Fact] + public async Task DistantUndefinedSymbol() + { + const string test = @" +class Test +{ + void Method() + { +#if OTHER_SYMBOL + return; +#endif + } +} +"; + + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols("FEATURE_RELEASE"); + + await VerifyCSharpDiagnosticAsync(context, test); + } + + [Fact] + public async Task LocallyDefinedSymbol() + { + const string test = @" +#define UNITTY_STANDALONE_OSX + +class Test +{ + void Method() + { +#if UNITTY_STANDALONE_OSX + return; +#endif + } +} +"; + + await VerifyCSharpDiagnosticAsync(test); + } + +} diff --git a/src/Microsoft.Unity.Analyzers.Tests/Infrastructure/AnalyzerVerificationContext.cs b/src/Microsoft.Unity.Analyzers.Tests/Infrastructure/AnalyzerVerificationContext.cs index 770aa05a..0c1c7d82 100644 --- a/src/Microsoft.Unity.Analyzers.Tests/Infrastructure/AnalyzerVerificationContext.cs +++ b/src/Microsoft.Unity.Analyzers.Tests/Infrastructure/AnalyzerVerificationContext.cs @@ -8,24 +8,27 @@ namespace Microsoft.Unity.Analyzers.Tests; -public readonly struct AnalyzerVerificationContext(ImmutableDictionary options, ImmutableArray filters, LanguageVersion languageVersion) +public readonly struct AnalyzerVerificationContext(ImmutableDictionary options, ImmutableArray filters, LanguageVersion languageVersion, ImmutableArray preprocessorSymbols) { public ImmutableDictionary Options { get; } = options; public ImmutableArray Filters { get; } = filters; public LanguageVersion LanguageVersion { get; } = languageVersion; + public ImmutableArray PreprocessorSymbols { get; } = preprocessorSymbols; // CS0414 - cf. IDE0051 public static AnalyzerVerificationContext Default = new( [], ["CS0414"], - LanguageVersion.Latest); + LanguageVersion.Latest, + []); public AnalyzerVerificationContext WithAnalyzerOption(string key, string value) { return new( Options.Add(key, value), Filters, - LanguageVersion); + LanguageVersion, + PreprocessorSymbols); } public AnalyzerVerificationContext WithAnalyzerFilter(string value) @@ -33,7 +36,8 @@ public AnalyzerVerificationContext WithAnalyzerFilter(string value) return new( Options, Filters.Add(value), - LanguageVersion); + LanguageVersion, + PreprocessorSymbols); } public AnalyzerVerificationContext WithLanguageVersion(LanguageVersion languageVersion) @@ -41,6 +45,16 @@ public AnalyzerVerificationContext WithLanguageVersion(LanguageVersion languageV return new( Options, Filters, - languageVersion); + languageVersion, + PreprocessorSymbols); + } + + public AnalyzerVerificationContext WithPreprocessorSymbols(params string[] symbols) + { + return new( + Options, + Filters, + LanguageVersion, + [.. symbols]); } } diff --git a/src/Microsoft.Unity.Analyzers.Tests/Infrastructure/DiagnosticVerifier.cs b/src/Microsoft.Unity.Analyzers.Tests/Infrastructure/DiagnosticVerifier.cs index e4e75c14..8224d539 100644 --- a/src/Microsoft.Unity.Analyzers.Tests/Infrastructure/DiagnosticVerifier.cs +++ b/src/Microsoft.Unity.Analyzers.Tests/Infrastructure/DiagnosticVerifier.cs @@ -366,7 +366,10 @@ private static Project CreateProject(AnalyzerVerificationContext context, string .AddProject(projectId, TestProjectName, TestProjectName, LanguageNames.CSharp); solution = UnityAssemblies().Aggregate(solution, (current, dll) => current.AddMetadataReference(projectId, MetadataReference.CreateFromFile(dll))); - solution = solution.WithProjectParseOptions(projectId, new CSharpParseOptions(context.LanguageVersion)); + + var parseOptions = new CSharpParseOptions(context.LanguageVersion) + .WithPreprocessorSymbols(context.PreprocessorSymbols); + solution = solution.WithProjectParseOptions(projectId, parseOptions); var count = 0; foreach (var source in sources) diff --git a/src/Microsoft.Unity.Analyzers/ConditionalCompilationSymbolTypo.cs b/src/Microsoft.Unity.Analyzers/ConditionalCompilationSymbolTypo.cs new file mode 100644 index 00000000..5722c4a2 --- /dev/null +++ b/src/Microsoft.Unity.Analyzers/ConditionalCompilationSymbolTypo.cs @@ -0,0 +1,257 @@ +/*-------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *-------------------------------------------------------------------------------------------*/ + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Text; +using Microsoft.Unity.Analyzers.Resources; + +namespace Microsoft.Unity.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class ConditionalCompilationSymbolTypoAnalyzer : DiagnosticAnalyzer +{ + private const string RuleId = "UNT0043"; + internal const string SuggestedSymbolPropertyName = "SuggestedSymbol"; + + internal static readonly DiagnosticDescriptor Rule = new( + id: RuleId, + title: Strings.ConditionalCompilationSymbolTypoDiagnosticTitle, + messageFormat: Strings.ConditionalCompilationSymbolTypoDiagnosticMessageFormat, + category: DiagnosticCategory.Correctness, + defaultSeverity: DiagnosticSeverity.Info, + isEnabledByDefault: true, + helpLinkUri: HelpLink.ForDiagnosticId(RuleId), + description: Strings.ConditionalCompilationSymbolTypoDiagnosticDescription); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); + + private const int LongSymbolLength = 8; + private const int MaxShortSymbolEditDistance = 1; + private const int MaxLongSymbolEditDistance = 2; + private const int MaxStackAllocEditDistanceBufferLength = 64; + + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + context.RegisterSyntaxTreeAction(AnalyzeSyntaxTree); + } + + private static void AnalyzeSyntaxTree(SyntaxTreeAnalysisContext context) + { + if (context.Tree.Options is not CSharpParseOptions parseOptions) + return; + + var knownSymbols = GetKnownSymbols(parseOptions); + if (knownSymbols.Length == 0) + return; + + var root = context.Tree.GetRoot(context.CancellationToken); + var knownSymbolSet = new HashSet(knownSymbols, StringComparer.Ordinal); + var localSymbols = GetLocalSymbols(root); + + foreach (var condition in GetDirectiveConditions(root)) + { + foreach (var identifier in condition.DescendantNodesAndSelf().OfType()) + { + context.CancellationToken.ThrowIfCancellationRequested(); + + var symbol = identifier.Identifier.ValueText; + if (string.IsNullOrEmpty(symbol)) + continue; + + if (knownSymbolSet.Contains(symbol) || localSymbols.Contains(symbol)) + continue; + + if (!TryFindClosestSymbol(symbol, knownSymbols, out var closestSymbol)) + continue; + + context.ReportDiagnostic(Diagnostic.Create( + Rule, + identifier.GetLocation(), + null, + ImmutableDictionary.Empty.Add(SuggestedSymbolPropertyName, closestSymbol), + symbol, + closestSymbol)); + } + } + } + + private static string[] GetKnownSymbols(CSharpParseOptions parseOptions) + { + return [.. parseOptions.PreprocessorSymbolNames + .Where(symbol => !string.IsNullOrWhiteSpace(symbol)) + .Distinct(StringComparer.Ordinal) + .OrderBy(symbol => symbol, StringComparer.Ordinal)]; + } + + private static HashSet GetLocalSymbols(SyntaxNode root) + { + var symbols = new HashSet(StringComparer.Ordinal); + foreach (var trivia in root.DescendantTrivia()) + { + if (trivia.GetStructure() is not DefineDirectiveTriviaSyntax defineDirective) + continue; + + var symbol = defineDirective.Name.ValueText; + if (!string.IsNullOrEmpty(symbol)) + symbols.Add(symbol); + } + + return symbols; + } + + private static IEnumerable GetDirectiveConditions(SyntaxNode root) + { + foreach (var trivia in root.DescendantTrivia()) + { + switch (trivia.GetStructure()) + { + case IfDirectiveTriviaSyntax ifDirective: + yield return ifDirective.Condition; + break; + case ElifDirectiveTriviaSyntax elifDirective: + yield return elifDirective.Condition; + break; + } + } + } + + private static bool TryFindClosestSymbol(string symbol, string[] candidates, [NotNullWhen(true)] out string? closestSymbol) + { + closestSymbol = null; + if (symbol.Length < 3) + return false; + + var maxDistance = GetMaxEditDistance(symbol); + var bestDistance = maxDistance + 1; + var bestLengthDifference = int.MaxValue; + + foreach (var candidate in candidates) + { + var lengthDifference = Math.Abs(symbol.Length - candidate.Length); + if (lengthDifference > maxDistance) + continue; + + var distance = GetEditDistance(symbol, candidate, maxDistance); + if (distance > maxDistance) + continue; + + if (distance > bestDistance) + continue; + + if (distance == bestDistance && lengthDifference >= bestLengthDifference) + continue; + + closestSymbol = candidate; + bestDistance = distance; + bestLengthDifference = lengthDifference; + } + + return closestSymbol != null; + } + + private static int GetMaxEditDistance(string symbol) + { + return symbol.Length >= LongSymbolLength ? MaxLongSymbolEditDistance : MaxShortSymbolEditDistance; + } + + private static int GetEditDistance(string source, string target, int maxDistance) + { + if (source.Length == 0) + return target.Length; + + if (target.Length == 0) + return source.Length; + + if (Math.Abs(source.Length - target.Length) > maxDistance) + return maxDistance + 1; + + var bufferLength = target.Length + 1; + var previous = bufferLength <= MaxStackAllocEditDistanceBufferLength ? stackalloc int[bufferLength] : new int[bufferLength]; + var current = bufferLength <= MaxStackAllocEditDistanceBufferLength ? stackalloc int[bufferLength] : new int[bufferLength]; + + for (var i = 0; i <= target.Length; i++) + previous[i] = i; + + for (var sourceIndex = 1; sourceIndex <= source.Length; sourceIndex++) + { + current[0] = sourceIndex; + var rowMinimum = current[0]; + + for (var targetIndex = 1; targetIndex <= target.Length; targetIndex++) + { + var substitutionCost = source[sourceIndex - 1] == target[targetIndex - 1] ? 0 : 1; + var deletion = previous[targetIndex] + 1; + var insertion = current[targetIndex - 1] + 1; + var substitution = previous[targetIndex - 1] + substitutionCost; + var distance = Math.Min(Math.Min(deletion, insertion), substitution); + + current[targetIndex] = distance; + rowMinimum = Math.Min(rowMinimum, distance); + } + + if (rowMinimum > maxDistance) + return maxDistance + 1; + + var nextPrevious = previous; + previous = current; + current = nextPrevious; + } + + return previous[target.Length]; + } + +} + +[ExportCodeFixProvider(LanguageNames.CSharp)] +public class ConditionalCompilationSymbolTypoCodeFix : CodeFixProvider +{ + public sealed override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(ConditionalCompilationSymbolTypoAnalyzer.Rule.Id); + + public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) + { + var diagnostic = context.Diagnostics.FirstOrDefault(); + if (diagnostic == null) + return Task.CompletedTask; + + if (!diagnostic.Properties.TryGetValue(ConditionalCompilationSymbolTypoAnalyzer.SuggestedSymbolPropertyName, out var suggestedSymbol) || + suggestedSymbol is not { Length: > 0 } replacement) + return Task.CompletedTask; + + context.RegisterCodeFix( + CodeAction.Create( + string.Format(Strings.ConditionalCompilationSymbolTypoCodeFixTitle, replacement), + ct => ReplaceSymbolAsync(context.Document, context.Span, replacement, ct), + FixableDiagnosticIds.Single()), // using DiagnosticId as equivalence key for BatchFixer + context.Diagnostics); + + return Task.CompletedTask; + } + + private static async Task ReplaceSymbolAsync(Document document, TextSpan span, string symbol, CancellationToken cancellationToken) + { + var text = await document + .GetTextAsync(cancellationToken) + .ConfigureAwait(false); + + return document.WithText(text.WithChanges(new TextChange(span, symbol))); + } +} diff --git a/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs b/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs index 6d47d9d4..73c05fb4 100644 --- a/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs +++ b/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs @@ -167,7 +167,43 @@ internal static string CodeStyleSuppressorJustification { return ResourceManager.GetString("CodeStyleSuppressorJustification", resourceCulture); } } - + + /// + /// Looks up a localized string similar to Replace with '{0}'. + /// + internal static string ConditionalCompilationSymbolTypoCodeFixTitle { + get { + return ResourceManager.GetString("ConditionalCompilationSymbolTypoCodeFixTitle", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Conditional compilation symbols that are very close to project-level preprocessor symbols may be typos and can make #if or #elif blocks compile unexpectedly.. + /// + internal static string ConditionalCompilationSymbolTypoDiagnosticDescription { + get { + return ResourceManager.GetString("ConditionalCompilationSymbolTypoDiagnosticDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Conditional compilation symbol '{0}' is not defined. Did you mean '{1}'?. + /// + internal static string ConditionalCompilationSymbolTypoDiagnosticMessageFormat { + get { + return ResourceManager.GetString("ConditionalCompilationSymbolTypoDiagnosticMessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Possible typo in conditional compilation symbol. + /// + internal static string ConditionalCompilationSymbolTypoDiagnosticTitle { + get { + return ResourceManager.GetString("ConditionalCompilationSymbolTypoDiagnosticTitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to Component '{0}' should not be instantiated directly.. /// diff --git a/src/Microsoft.Unity.Analyzers/Resources/Strings.resx b/src/Microsoft.Unity.Analyzers/Resources/Strings.resx index 5496cc1c..41865e2e 100644 --- a/src/Microsoft.Unity.Analyzers/Resources/Strings.resx +++ b/src/Microsoft.Unity.Analyzers/Resources/Strings.resx @@ -676,4 +676,18 @@ Mesh array property accessed in loop + + Conditional compilation symbols that are very close to project-level preprocessor symbols may be typos and can make #if or #elif blocks compile unexpectedly. + + + Replace with '{0}' + {0} is the suggested preprocessor symbol name + + + Conditional compilation symbol '{0}' is not defined. Did you mean '{1}'? + {0} is the symbol used in source, {1} is the closest known symbol + + + Possible typo in conditional compilation symbol +