fix position diagnostics

dotnet · RikkiGibson · Apr 25, 2023 · Apr 13, 2023 · Apr 13, 2023 · Apr 15, 2023
commit f09ecb9277e266f55b6d8591758a0fd6e2a3fa6c
@@ -7523,11 +7523,14 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="ERR_InterceptorCharacterOutOfRange" xml:space="preserve">
     <value>The given line is '{0}' characters long, which is fewer than the provided character number '{1}'.</value>
   </data>
+  <data name="ERR_InterceptorLineCharacterMustBePositive" xml:space="preserve">
+    <value>Line and character numbers provided to InterceptsLocationAttribute must be positive.</value>
+  </data>
   <data name="ERR_InterceptorPositionBadToken" xml:space="preserve">
     <value>The provided line and character number does not refer to an interceptable method name, but rather to token '{0}'.</value>
   </data>
   <data name="ERR_InterceptorMustReferToStartOfTokenPosition" xml:space="preserve">
-    <value>The provided character number does not refer to the start of method name token '{0}'. Consider using character number '{1}' instead.</value>
+    <value>The provided line and character number does not refer to the start of token '{0}'. Did you mean to use line '{1}' and character '{2}'?</value>
   </data>
   <data name="ERR_InterceptorSignatureMismatch" xml:space="preserve">
     <value>Cannot intercept method '{0}' with interceptor '{1}' because the signatures do not match.</value>

@@ -2213,6 +2213,7 @@ internal enum ErrorCode
         WRN_InterceptorSignatureMismatch = 27017,
         ERR_InterceptorNotAccessible = 27018,
         ERR_InterceptorScopedMismatch = 27019,
+        ERR_InterceptorLineCharacterMustBePositive = 27020,
 
         #endregion
 

@@ -587,6 +587,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
                 case ErrorCode.ERR_InterceptorMustNotHaveThisParameter:
                 case ErrorCode.ERR_DuplicateInterceptor:
                 case ErrorCode.WRN_InterceptorSignatureMismatch:
+                case ErrorCode.ERR_InterceptorNotAccessible:
                 case ErrorCode.ERR_InterceptorScopedMismatch:
                     // Update src\EditorFeatures\CSharp\LanguageServer\CSharpLspBuildOnlyDiagnostics.cs
                     // whenever new values are added here.
@@ -2330,6 +2331,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
                 case ErrorCode.ERR_InterceptorFilePathCannotBeNull:
                 case ErrorCode.ERR_InterceptorNameNotInvoked:
                 case ErrorCode.ERR_InterceptorNonUniquePath:
+                case ErrorCode.ERR_InterceptorLineCharacterMustBePositive:
                 case ErrorCode.ERR_ConstantValueOfTypeExpected:
                 case ErrorCode.ERR_UnsupportedPrimaryConstructorParameterCapturingRefAny:
                     return false;

@@ -950,7 +950,8 @@ private void DecodeInterceptsLocationAttribute(DecodeWellKnownAttributeArguments
         {
             Debug.Assert(arguments.AttributeSyntaxOpt is object);
             Debug.Assert(!arguments.Attribute.HasErrors);
-            var attributeArguments = arguments.Attribute.CommonConstructorArguments;
+            var attributeData = arguments.Attribute;
+            var attributeArguments = attributeData.CommonConstructorArguments;
             if (attributeArguments is not [
                 { Type.SpecialType: SpecialType.System_String },
                 { Kind: not TypedConstantKind.Array, Value: int lineNumberOneBased },
@@ -961,12 +962,16 @@ private void DecodeInterceptsLocationAttribute(DecodeWellKnownAttributeArguments
             }
 
             var diagnostics = (BindingDiagnosticBag)arguments.Diagnostics;
-            var attributeLocation = arguments.AttributeSyntaxOpt.Location;
+            var attributeSyntax = arguments.AttributeSyntaxOpt;
+            var attributeLocation = attributeSyntax.Location;
+            const int filePathParameterIndex = 0;
+            const int lineNumberParameterIndex = 1;
+            const int characterNumberParameterIndex = 2;
 
             var filePath = (string?)attributeArguments[0].Value;
             if (filePath is null)
             {
-                diagnostics.Add(ErrorCode.ERR_InterceptorFilePathCannotBeNull, attributeLocation);
+                diagnostics.Add(ErrorCode.ERR_InterceptorFilePathCannotBeNull, attributeData.GetAttributeArgumentSyntaxLocation(filePathParameterIndex, attributeSyntax));
                 return;
             }
 
@@ -983,46 +988,54 @@ private void DecodeInterceptsLocationAttribute(DecodeWellKnownAttributeArguments
             }
 
             var syntaxTrees = DeclaringCompilation.SyntaxTrees;
+            // PROTOTYPE(ic): consider avoiding an array allocation here, on the assumption that 1 matching tree is the success (common) case, 0 is the most common error case, and 2 or more is much more rare.
             var matchingTrees = syntaxTrees.WhereAsArray(static (tree, filePath) => tree.FilePath == filePath, filePath);
             if (matchingTrees is [])
             {
                 var suffixMatch = syntaxTrees.FirstOrDefault(static (tree, filePath) => tree.FilePath.EndsWith(filePath), filePath);
                 if (suffixMatch != null)
                 {
-                    diagnostics.Add(ErrorCode.ERR_InterceptorPathNotInCompilationWithCandidate, attributeLocation, filePath, suffixMatch.FilePath);
+                    diagnostics.Add(ErrorCode.ERR_InterceptorPathNotInCompilationWithCandidate, attributeData.GetAttributeArgumentSyntaxLocation(filePathParameterIndex, attributeSyntax), filePath, suffixMatch.FilePath);
                 }
                 else
                 {
-                    diagnostics.Add(ErrorCode.ERR_InterceptorPathNotInCompilation, attributeLocation, filePath);
+                    diagnostics.Add(ErrorCode.ERR_InterceptorPathNotInCompilation, attributeData.GetAttributeArgumentSyntaxLocation(filePathParameterIndex, attributeSyntax), filePath);
                 }
 
                 return;
             }
 
             if (matchingTrees is not [var matchingTree])
             {
-                diagnostics.Add(ErrorCode.ERR_InterceptorNonUniquePath, attributeLocation, filePath);
+                diagnostics.Add(ErrorCode.ERR_InterceptorNonUniquePath, attributeData.GetAttributeArgumentSyntaxLocation(filePathParameterIndex, attributeSyntax), filePath);
                 return;
             }
 
             // Internally, line and character numbers are 0-indexed, but when they appear in code or diagnostic messages, they are 1-indexed.
-            // PROTOTYPE(ic): test with zero or negative display line/character numbers.
             int lineNumberZeroBased = lineNumberOneBased - 1;
             int characterNumberZeroBased = characterNumberOneBased - 1;
 
+            if (lineNumberZeroBased < 0 || characterNumberZeroBased < 0)
+            {
+                var location = attributeData.GetAttributeArgumentSyntaxLocation(lineNumberZeroBased < 0 ? lineNumberParameterIndex : characterNumberParameterIndex, attributeSyntax);
+                diagnostics.Add(ErrorCode.ERR_InterceptorLineCharacterMustBePositive, location);
+                return;
+            }
+
             var referencedLines = matchingTree.GetText().Lines;
             var referencedLineCount = referencedLines.Count;
+
             if (lineNumberZeroBased >= referencedLineCount)
             {
-                diagnostics.Add(ErrorCode.ERR_InterceptorLineOutOfRange, attributeLocation, referencedLineCount, lineNumberOneBased);
+                diagnostics.Add(ErrorCode.ERR_InterceptorLineOutOfRange, attributeData.GetAttributeArgumentSyntaxLocation(lineNumberParameterIndex, attributeSyntax), referencedLineCount, lineNumberOneBased);
                 return;
             }
 
             var line = referencedLines[lineNumberZeroBased];
             var lineLength = line.End - line.Start;
             if (characterNumberZeroBased >= lineLength)
             {
-                diagnostics.Add(ErrorCode.ERR_InterceptorCharacterOutOfRange, attributeLocation, lineLength, characterNumberOneBased);
+                diagnostics.Add(ErrorCode.ERR_InterceptorCharacterOutOfRange, attributeData.GetAttributeArgumentSyntaxLocation(characterNumberParameterIndex, attributeSyntax), lineLength, characterNumberOneBased);
                 return;
             }
 
@@ -1046,12 +1059,11 @@ private void DecodeInterceptsLocationAttribute(DecodeWellKnownAttributeArguments
                     return;
             }
 
-            // Did they actually refer to the start of the token, not the middle, or in leading trivia?
-            var tokenPositionDifference = referencedPosition - referencedToken.Span.Start;
-            if (tokenPositionDifference != 0)
+            // Did they actually refer to the start of the token, not the middle, or in trivia?
+            if (referencedPosition != referencedToken.Span.Start)
             {
-                // PROTOTYPE(ic): when a token's leading trivia spans multiple lines, this doesn't suggest a valid position.
-                diagnostics.Add(ErrorCode.ERR_InterceptorMustReferToStartOfTokenPosition, attributeLocation, referencedToken.Text, characterNumberOneBased - tokenPositionDifference);
+                var linePositionZeroBased = referencedToken.GetLocation().GetLineSpan().StartLinePosition;
+                diagnostics.Add(ErrorCode.ERR_InterceptorMustReferToStartOfTokenPosition, attributeLocation, referencedToken.Text, linePositionZeroBased.Line + 1, linePositionZeroBased.Character + 1);
                 return;
             }