diff --git a/doc/UNT0043.md b/doc/UNT0043.md index 8ab2407..9221847 100644 --- a/doc/UNT0043.md +++ b/doc/UNT0043.md @@ -10,7 +10,7 @@ Mistyped conditional compilation symbols in `#if` and `#elif` directives are tre #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. +If the project defines `UNITY_STANDALONE_OSX` and no other symbol is an equally plausible correction, `UNITTY_STANDALONE_OSX` is reported as a likely typo. The analyzer gets the currently enabled symbols from Roslyn's parse options for the project; it does not maintain a catalog of Unity platform symbols. The same applies to custom project symbols: @@ -22,6 +22,35 @@ The same applies to custom project symbols: If the project defines `FEATURE_RELEASE`, this is reported as a likely typo. +## Avoiding false positives + +An undefined symbol can be intentional: its code may target another platform, Unity version, or optional feature. The analyzer only reports an undefined symbol when exactly one enabled project symbol differs by a single insertion, deletion, substitution, or adjacent transposition. Symbols shorter than three characters and symbols declared with a local `#define` are not reported. + +Numeric components must remain unchanged. For example, `FEATURE_1` is not corrected to `FEATURE_2` or `FEATURE_12`. Splitting or merging numeric components is also excluded. + +Well-formed Unity version symbols are never reported, regardless of whether they are enabled in the current project: + +- `UNITY_X` +- `UNITY_X_Y` +- `UNITY_X_Y_Z` +- `UNITY_X_Y_OR_NEWER` + +Here, `X`, `Y`, and `Z` are numeric components. For example, these conditions are intentional even in a Unity 6.3 project targeting Windows: + +```csharp +#if UNITY_6000_5_OR_NEWER + UseNewerUnityApi(); +#endif + +#if UNITY_PS4 || UNITY_PS5 + UsePlayStationFeatures(); +#endif +``` + +The analyzer does not change version numbers or suggest `UNITY_64` for `UNITY_PS4`. Spelling errors such as `UNITY_EDTIOR` or `UNITY_6000_3_OR_NEWRE` can still be reported if the intended symbol is enabled and the correction is unambiguous. + +This is a conservative heuristic, not validation of all possible symbols. It can miss typos for inactive platforms or optional features, and similarly named optional symbols can still produce false positives. Review the intended condition before applying a fix. + ## Solution Use the intended symbol name: diff --git a/src/Microsoft.Unity.Analyzers.Tests/ConditionalCompilationSymbolTypoTests.cs b/src/Microsoft.Unity.Analyzers.Tests/ConditionalCompilationSymbolTypoTests.cs index 3cacf0e..3201dac 100644 --- a/src/Microsoft.Unity.Analyzers.Tests/ConditionalCompilationSymbolTypoTests.cs +++ b/src/Microsoft.Unity.Analyzers.Tests/ConditionalCompilationSymbolTypoTests.cs @@ -69,7 +69,7 @@ void Method() .WithArguments("UNITTY_STANDALONE_OSX", "UNITY_STANDALONE_OSX"); var context = AnalyzerVerificationContext.Default - .WithPreprocessorSymbols("UNITY_EDITOR", "UNITY_STANDALONE_WIN", "UNITY_STANDALONE_LINUX", "UNITY_STANDALONE_OSX"); + .WithPreprocessorSymbols("UNITY_EDITOR", "UNITY_EDITOR_OSX", "UNITY_STANDALONE", "UNITY_STANDALONE_OSX", "UNITY_64"); await VerifyCSharpDiagnosticAsync(context, test, diagnostic); @@ -115,15 +115,129 @@ void Method() await VerifyCSharpDiagnosticAsync(context, test, diagnostic); } + [Theory] + [InlineData("EATURE_RELEASE", "FEATURE_RELEASE")] + [InlineData("FEATURE_RELESE", "FEATURE_RELEASE")] + [InlineData("FEATURE_RELEAS", "FEATURE_RELEASE")] + [InlineData("FFEATURE_RELEASE", "FEATURE_RELEASE")] + [InlineData("FEATTURE_RELEASE", "FEATURE_RELEASE")] + [InlineData("FEATURE_RELEASEE", "FEATURE_RELEASE")] + [InlineData("GEATURE_RELEASE", "FEATURE_RELEASE")] + [InlineData("FEATURE_RELEAZE", "FEATURE_RELEASE")] + [InlineData("FEATURE_RELEASX", "FEATURE_RELEASE")] + [InlineData("EFATURE_RELEASE", "FEATURE_RELEASE")] + [InlineData("FETAURE_RELEASE", "FEATURE_RELEASE")] + [InlineData("FEATURE_RELEAES", "FEATURE_RELEASE")] + [InlineData("UNITY_EDTIOR", "UNITY_EDITOR")] + [InlineData("UNITY_6000_3_OR_NEWRE", "UNITY_6000_3_OR_NEWER")] + [InlineData("UNITY_6000__3_OR_NEWER", "UNITY_6000_3_OR_NEWER")] + [InlineData("UNITY_6000_3_OR_NEWE", "UNITY_6000_3_OR_NEWER")] + [InlineData("UNITY_6000_3_OR_NEWER_", "UNITY_6000_3_OR_NEWER")] + [InlineData("FEATURE_12_V2_RELEAS", "FEATURE_12_V2_RELEASE")] + [InlineData("FEATURE_1A2", "FEATURE_1B2")] + [InlineData("ACB", "ABC")] + public async Task SingleTypo(string symbol, string definedSymbol) + { + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols(definedSymbol); + + var diagnostic = ExpectDiagnostic() + .WithLocation(6, 5) + .WithArguments(symbol, definedSymbol); + + await VerifyCSharpDiagnosticAsync(context, Source(symbol), diagnostic); + await VerifyCSharpFixAsync(context, Source(symbol), Source(definedSymbol)); + } + + [Theory] + [InlineData("FEATURE_RELXXSE")] + [InlineData("FFEATURE_RELEASEE")] + [InlineData("FEATURE_RELEASXX")] + [InlineData("FETAURE_RELEAS")] + public async Task MultipleEdits(string symbol) + { + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols("FEATURE_RELEASE"); + + await VerifyCSharpDiagnosticAsync(context, Source(symbol)); + } + + [Theory] + [InlineData("FEATURE_1", "FEATURE_2")] + [InlineData("FEATURE_1", "FEATURE_12")] + [InlineData("FEATURE_12", "FEATURE_1")] + [InlineData("FEATURE_12", "FEATURE_21")] + [InlineData("FEATURE_01", "FEATURE_1")] + [InlineData("FEATURE", "FEATURE1")] + [InlineData("FEATURE1", "FEATURE")] + [InlineData("FEATURE_1_2", "FEATURE_12")] + [InlineData("FEATURE_12", "FEATURE_1_2")] + [InlineData("FEATURE_1A2", "FEATURE_12")] + [InlineData("FEATURE_V2_X3", "FEATURE_V2_X4")] + [InlineData("FEATURE_V2_X3", "FEATURE_V2_X")] + [InlineData("FEATURE_V2_X", "FEATURE_V2_X3")] + public async Task DifferentNumericComponents(string symbol, string definedSymbol) + { + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols(definedSymbol); + + await VerifyCSharpDiagnosticAsync(context, Source(symbol)); + } + + [Theory] + [InlineData("UNITY_5")] + [InlineData("UNITY_2019_4")] + [InlineData("UNITY_2019_4_40")] + [InlineData("UNITY_6000_2")] + [InlineData("UNITY_6000_3_8")] + [InlineData("UNITY_6000_4_OR_NEWER")] + [InlineData("UNITY_6000_5_OR_NEWER")] + [InlineData("UNITY_9999_9_OR_NEWER")] + public async Task InactiveUnityVersionSymbol(string symbol) + { + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols("UNITY_6000", "UNITY_6000_3", "UNITY_6000_3_9", + "UNITY_6000_0_OR_NEWER", "UNITY_6000_1_OR_NEWER", "UNITY_6000_2_OR_NEWER", "UNITY_6000_3_OR_NEWER"); + + await VerifyCSharpDiagnosticAsync(context, Source(symbol)); + } + + [Theory] + [InlineData("UNITY_6000")] + [InlineData("UNITY_6000_4")] + [InlineData("UNITY_6000_4_1")] + [InlineData("UNITY_6000_4_OR_NEWER")] + public async Task UnityVersionSymbolResemblingCustomSymbol(string symbol) + { + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols(symbol + "_"); + + await VerifyCSharpDiagnosticAsync(context, Source(symbol)); + } + + [Theory] + [InlineData("UNITY_PS4 || UNITY_PS5")] + [InlineData("UNITY_STANDALONE_OSX || UNITY_STANDALONE_LINUX")] + [InlineData("UNITY_EDITOR || UNITY_STANDALONE || UNITY_PS4 || UNITY_PS5 || UNITY_WSA || UNITY_ANDROID || UNITY_IOS || UNITY_TVOS || UNITY_VISIONOS")] + public async Task InactivePlatformSymbols(string condition) + { + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols("UNITY_EDITOR", "UNITY_EDITOR_WIN", "UNITY_STANDALONE", "UNITY_STANDALONE_WIN", "UNITY_64"); + + await VerifyCSharpDiagnosticAsync(context, Source(condition)); + } + [Fact] - public async Task ProjectSymbolTypo() + public async Task InactiveVersionSymbolsInElif() { const string test = @" class Test { void Method() { -#if FEATURE_RELEAS +#if UNITY_PS4 || UNITY_PS5 + return; +#elif !UNITY_6000_4_OR_NEWER && !UNITY_6000_5_OR_NEWER return; #endif } @@ -131,13 +245,57 @@ void Method() "; var context = AnalyzerVerificationContext.Default - .WithPreprocessorSymbols("FEATURE_RELEASE"); + .WithPreprocessorSymbols("UNITY_STANDALONE", "UNITY_STANDALONE_WIN", "UNITY_64", "UNITY_6000_3_OR_NEWER"); + + await VerifyCSharpDiagnosticAsync(context, test); + } + + [Theory] + [InlineData("FEATURE_RELEASX", "FEATURE_RELEASE", "FEATURE_RELEASY")] + [InlineData("FEATURE_RELEAS", "FEATURE_RELEASE", "FEATURE_RELEAZ")] + [InlineData("UNITY_EDTIOR", "UNITY_EDITOR", "UNITY_EDTIOS")] + public async Task AmbiguousSymbol(string symbol, string firstCandidate, string secondCandidate) + { + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols(firstCandidate, secondCandidate); + + await VerifyCSharpDiagnosticAsync(context, Source(symbol)); + + context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols(secondCandidate, firstCandidate); + + await VerifyCSharpDiagnosticAsync(context, Source(symbol)); + } + + [Fact] + public async Task DuplicateDefinedSymbolsAreNotAmbiguous() + { + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols("FEATURE_RELEASE", "FEATURE_RELEASE"); var diagnostic = ExpectDiagnostic() .WithLocation(6, 5) .WithArguments("FEATURE_RELEAS", "FEATURE_RELEASE"); - await VerifyCSharpDiagnosticAsync(context, test, diagnostic); + await VerifyCSharpDiagnosticAsync(context, Source("FEATURE_RELEAS"), diagnostic); + } + + [Fact] + public async Task TypoForInactivePlatform() + { + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols("UNITY_EDITOR", "UNITY_EDITOR_WIN", "UNITY_STANDALONE", "UNITY_STANDALONE_WIN", "UNITY_64"); + + await VerifyCSharpDiagnosticAsync(context, Source("UNITTY_STANDALONE_OSX")); + } + + [Fact] + public async Task ShortSymbol() + { + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols("ABC"); + + await VerifyCSharpDiagnosticAsync(context, Source("AB")); } [Fact] @@ -199,7 +357,24 @@ void Method() } "; - await VerifyCSharpDiagnosticAsync(test); + var context = AnalyzerVerificationContext.Default + .WithPreprocessorSymbols("UNITY_STANDALONE_OSX"); + + await VerifyCSharpDiagnosticAsync(context, test); } + private static string Source(string condition) + { + return $@" +class Test +{{ + void Method() + {{ +#if {condition} + return; +#endif + }} +}} +"; + } } diff --git a/src/Microsoft.Unity.Analyzers/ConditionalCompilationSymbolTypo.cs b/src/Microsoft.Unity.Analyzers/ConditionalCompilationSymbolTypo.cs index 5722c4a..69c5e53 100644 --- a/src/Microsoft.Unity.Analyzers/ConditionalCompilationSymbolTypo.cs +++ b/src/Microsoft.Unity.Analyzers/ConditionalCompilationSymbolTypo.cs @@ -8,6 +8,7 @@ using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; @@ -39,10 +40,8 @@ public class ConditionalCompilationSymbolTypoAnalyzer : DiagnosticAnalyzer 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; + // UNITY_X, UNITY_X_Y, UNITY_X_Y_Z, and UNITY_X_Y_OR_NEWER. + private static readonly Regex UnityVersionSymbolRegex = new(@"\AUNITY_[0-9]+(?:_[0-9]+(?:_[0-9]+|_OR_NEWER)?)?\z", RegexOptions.Compiled); public override void Initialize(AnalysisContext context) { @@ -57,12 +56,12 @@ private static void AnalyzeSyntaxTree(SyntaxTreeAnalysisContext context) if (context.Tree.Options is not CSharpParseOptions parseOptions) return; - var knownSymbols = GetKnownSymbols(parseOptions); - if (knownSymbols.Length == 0) + var definedSymbols = GetDefinedSymbols(parseOptions); + if (definedSymbols.Length == 0) return; var root = context.Tree.GetRoot(context.CancellationToken); - var knownSymbolSet = new HashSet(knownSymbols, StringComparer.Ordinal); + var definedSymbolSet = new HashSet(definedSymbols, StringComparer.Ordinal); var localSymbols = GetLocalSymbols(root); foreach (var condition in GetDirectiveConditions(root)) @@ -75,29 +74,28 @@ private static void AnalyzeSyntaxTree(SyntaxTreeAnalysisContext context) if (string.IsNullOrEmpty(symbol)) continue; - if (knownSymbolSet.Contains(symbol) || localSymbols.Contains(symbol)) + if (definedSymbolSet.Contains(symbol) || localSymbols.Contains(symbol) || UnityVersionSymbolRegex.IsMatch(symbol)) continue; - if (!TryFindClosestSymbol(symbol, knownSymbols, out var closestSymbol)) + if (!TryFindSuggestedSymbol(symbol, definedSymbols, out var suggestedSymbol)) continue; context.ReportDiagnostic(Diagnostic.Create( Rule, identifier.GetLocation(), null, - ImmutableDictionary.Empty.Add(SuggestedSymbolPropertyName, closestSymbol), + ImmutableDictionary.Empty.Add(SuggestedSymbolPropertyName, suggestedSymbol), symbol, - closestSymbol)); + suggestedSymbol)); } } } - private static string[] GetKnownSymbols(CSharpParseOptions parseOptions) + private static string[] GetDefinedSymbols(CSharpParseOptions parseOptions) { return [.. parseOptions.PreprocessorSymbolNames .Where(symbol => !string.IsNullOrWhiteSpace(symbol)) - .Distinct(StringComparer.Ordinal) - .OrderBy(symbol => symbol, StringComparer.Ordinal)]; + .Distinct(StringComparer.Ordinal)]; } private static HashSet GetLocalSymbols(SyntaxNode root) @@ -132,91 +130,84 @@ private static IEnumerable GetDirectiveConditions(SyntaxNode r } } - private static bool TryFindClosestSymbol(string symbol, string[] candidates, [NotNullWhen(true)] out string? closestSymbol) + private static bool TryFindSuggestedSymbol(string symbol, string[] candidates, [NotNullWhen(true)] out string? suggestedSymbol) { - closestSymbol = null; + suggestedSymbol = 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) + if (!IsSingleEdit(symbol.AsSpan(), candidate.AsSpan()) || + !HaveSameNumericComponents(symbol.AsSpan(), candidate.AsSpan())) continue; - if (distance > bestDistance) - continue; - - if (distance == bestDistance && lengthDifference >= bestLengthDifference) - continue; + if (suggestedSymbol != null) + { + suggestedSymbol = null; + return false; + } - closestSymbol = candidate; - bestDistance = distance; - bestLengthDifference = lengthDifference; + suggestedSymbol = candidate; } - return closestSymbol != null; + return suggestedSymbol != null; } - private static int GetMaxEditDistance(string symbol) + private static bool IsSingleEdit(ReadOnlySpan source, ReadOnlySpan target) { - return symbol.Length >= LongSymbolLength ? MaxLongSymbolEditDistance : MaxShortSymbolEditDistance; - } + if (Math.Abs(source.Length - target.Length) > 1) + return false; - private static int GetEditDistance(string source, string target, int maxDistance) - { - if (source.Length == 0) - return target.Length; + var index = 0; + while (index < source.Length && index < target.Length && source[index] == target[index]) + index++; - if (target.Length == 0) - return source.Length; + if (source.Length != target.Length) + { + return source.Length > target.Length + ? source.Slice(index + 1).SequenceEqual(target.Slice(index)) + : source.Slice(index).SequenceEqual(target.Slice(index + 1)); + } - if (Math.Abs(source.Length - target.Length) > maxDistance) - return maxDistance + 1; + if (index == source.Length) + return false; - 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]; + if (source.Slice(index + 1).SequenceEqual(target.Slice(index + 1))) + return true; - for (var i = 0; i <= target.Length; i++) - previous[i] = i; + return index + 1 < source.Length && + source[index] == target[index + 1] && + source[index + 1] == target[index] && + source.Slice(index + 2).SequenceEqual(target.Slice(index + 2)); + } - for (var sourceIndex = 1; sourceIndex <= source.Length; sourceIndex++) + private static bool HaveSameNumericComponents(ReadOnlySpan source, ReadOnlySpan target) + { + var sourceIndex = 0; + var targetIndex = 0; + while (sourceIndex < source.Length || targetIndex < target.Length) { - current[0] = sourceIndex; - var rowMinimum = current[0]; + var sourceComponent = GetNextNumericComponent(source, ref sourceIndex); + var targetComponent = GetNextNumericComponent(target, ref targetIndex); + if (!sourceComponent.SequenceEqual(targetComponent)) + return false; + } - 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); - } + return true; + } - if (rowMinimum > maxDistance) - return maxDistance + 1; + private static ReadOnlySpan GetNextNumericComponent(ReadOnlySpan symbol, ref int index) + { + while (index < symbol.Length && !char.IsDigit(symbol[index])) + index++; - var nextPrevious = previous; - previous = current; - current = nextPrevious; - } + var start = index; + while (index < symbol.Length && char.IsDigit(symbol[index])) + index++; - return previous[target.Length]; + return symbol.Slice(start, index - start); } - } [ExportCodeFixProvider(LanguageNames.CSharp)]