Interceptors: address duplicate interceptions feedback, handle nameof…

…, add test coverage (#67971)

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -7592,4 +7592,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="WRN_NullabilityMismatchInReturnTypeOnInterceptor_Title" xml:space="preserve">
     <value>Nullability of reference types in return type doesn't match interceptable method.</value>
   </data>
+  <data name="ERR_InterceptorCannotInterceptNameof" xml:space="preserve">
+    <value>A nameof operator cannot be intercepted.</value>
+  </data>
 </root>

src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs

@@ -2288,7 +2288,7 @@ internal void AddInterception(string filePath, int line, int character, Location
                 factoryArgument: (AttributeLocation: attributeLocation, Interceptor: interceptor));
         }
 
-        internal (Location AttributeLocation, MethodSymbol Interceptor)? TryGetInterceptor(Location? callLocation, BindingDiagnosticBag diagnostics)
+        internal (Location AttributeLocation, MethodSymbol Interceptor)? TryGetInterceptor(Location? callLocation)
         {
             if (_interceptions is null || callLocation is null)
             {
@@ -2306,10 +2306,9 @@ internal void AddInterception(string filePath, int line, int character, Location
                     return oneInterception;
                 }
 
-                // We don't normally reach this branch in batch compilation, because we would have already reported an error after the declaration phase.
-                // One scenario where we may reach this is when validating used assemblies, which performs lowering of method bodies even if declaration errors would be reported.
-                // See 'CSharpCompilation.GetCompleteSetOfUsedAssemblies'.
-                diagnostics.Add(ErrorCode.ERR_ModuleEmitFailure, callLocation, this.SourceModule.Name, new LocalizableResourceString(nameof(CSharpResources.ERR_DuplicateInterceptor), CodeAnalysisResources.ResourceManager, typeof(CodeAnalysisResources)));
+                // Duplicate interceptors is an error in the declaration phase.
+                // This method is only expected to be called if no such errors are present.
+                throw ExceptionUtilities.Unreachable();
             }
 
             return null;
@@ -3274,8 +3273,6 @@ internal override bool CompileMethods(
             bool hasDeclarationErrors = !FilterAndAppendDiagnostics(diagnostics, GetDiagnostics(CompilationStage.Declare, true, cancellationToken), excludeDiagnostics, cancellationToken);
             excludeDiagnostics?.Free();
 
-            hasDeclarationErrors |= CheckDuplicateInterceptions(diagnostics);
-
             // TODO (tomat): NoPIA:
             // EmbeddedSymbolManager.MarkAllDeferredSymbolsAsReferenced(this)
 
@@ -3416,8 +3413,8 @@ private bool CheckDuplicateFilePaths(DiagnosticBag diagnostics)
             return visitor.CheckDuplicateFilePathsAndFree(SyntaxTrees, GlobalNamespace);
         }
 
-        /// <returns><see langword="true"/> if duplicate interceptors are present in the compilation. Otherwise, <see langword="false" />.</returns>
-        private bool CheckDuplicateInterceptions(DiagnosticBag diagnostics)
+        /// <returns><see langword="true"/> if duplicate interceptions are present in the compilation. Otherwise, <see langword="false" />.</returns>
+        internal bool CheckDuplicateInterceptions(BindingDiagnosticBag diagnostics)
         {
             if (_interceptions is null)
             {

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

@@ -120,10 +120,7 @@ public static void CompileMethodBodies(
             Debug.Assert(diagnostics != null);
             Debug.Assert(diagnostics.DiagnosticBag != null);
 
-            // PROTOTYPE(ic):
-            // - Move check for duplicate interceptions in here.
-            // - Change lowering to throw on duplicates.
-            // - Test no duplicate error given when emitting a ref assembly.
+            hasDeclarationErrors |= compilation.CheckDuplicateInterceptions(diagnostics);
 
             if (compilation.PreviousSubmission != null)
             {

src/Compilers/CSharp/Portable/Errors/ErrorCode.cs

@@ -2216,6 +2216,7 @@ internal enum ErrorCode
         ERR_InterceptorLineCharacterMustBePositive = 27020,
         WRN_NullabilityMismatchInReturnTypeOnInterceptor = 27021,
         WRN_NullabilityMismatchInParameterTypeOnInterceptor = 27022,
+        ERR_InterceptorCannotInterceptNameof = 27023,
 
         #endregion
 

src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs

@@ -595,6 +595,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
                 case ErrorCode.ERR_InterceptorScopedMismatch:
                 case ErrorCode.WRN_NullabilityMismatchInReturnTypeOnInterceptor:
                 case ErrorCode.WRN_NullabilityMismatchInParameterTypeOnInterceptor:
+                case ErrorCode.ERR_InterceptorCannotInterceptNameof:
                     // Update src\EditorFeatures\CSharp\LanguageServer\CSharpLspBuildOnlyDiagnostics.cs
                     // whenever new values are added here.
                     return true;

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs

@@ -222,6 +222,16 @@ private PEModuleBuilder? EmitModule
 
         private BoundExpression? VisitExpressionImpl(BoundExpression node)
         {
+            if (node is BoundNameOfOperator nameofOperator)
+            {
+                Debug.Assert(!nameofOperator.WasCompilerGenerated);
+                var nameofIdentiferSyntax = (IdentifierNameSyntax)((InvocationExpressionSyntax)nameofOperator.Syntax).Expression;
+                if (this._compilation.TryGetInterceptor(nameofIdentiferSyntax.Location) is not null)
+                {
+                    this._diagnostics.Add(ErrorCode.ERR_InterceptorCannotInterceptNameof, nameofIdentiferSyntax.Location);
+                }
+            }
+
             ConstantValue? constantValue = node.ConstantValueOpt;
             if (constantValue != null)
             {

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs

@@ -145,7 +145,7 @@ private void InterceptCallAndAdjustArguments(
             // Add assertions for the possible shapes of calls which could come through this method.
             // When the BoundCall shape changes in the future, force developer to decide what to do here.
 
-            if (this._compilation.TryGetInterceptor(interceptableLocation, _diagnostics) is not var (attributeLocation, interceptor))
+            if (this._compilation.TryGetInterceptor(interceptableLocation) is not var (attributeLocation, interceptor))
             {
                 // The call was not intercepted.
                 return;

src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbolWithAttributes.cs

@@ -13,6 +13,7 @@
 using System.Threading;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Shared.Collections;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp.Symbols
@@ -988,9 +989,28 @@ 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 [])
+            SyntaxTree? matchingTree = null;
+            // PROTOTYPE(ic): we need to resolve the paths before comparing (i.e. respect /pathmap).
+            // At that time, we should look at caching the resolved paths for the trees in a set (or maybe a Map<string, OneOrMany<SyntaxTree>>).
+            // so we can reduce the cost of these checks.
+            foreach (var tree in syntaxTrees)
+            {
+                if (tree.FilePath == filePath)
+                {
+                    if (matchingTree == null)
+                    {
+                        matchingTree = tree;
+                        // need to keep searching in case we find another tree with the same path
+                    }
+                    else
+                    {
+                        diagnostics.Add(ErrorCode.ERR_InterceptorNonUniquePath, attributeData.GetAttributeArgumentSyntaxLocation(filePathParameterIndex, attributeSyntax), filePath);
+                        return;
+                    }
+                }
+            }
+
+            if (matchingTree == null)
             {
                 var suffixMatch = syntaxTrees.FirstOrDefault(static (tree, filePath) => tree.FilePath.EndsWith(filePath), filePath);
                 if (suffixMatch != null)
@@ -1005,12 +1025,6 @@ private void DecodeInterceptsLocationAttribute(DecodeWellKnownAttributeArguments
                 return;
             }
 
-            if (matchingTrees is not [var matchingTree])
-            {
-                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.
             int lineNumberZeroBased = lineNumberOneBased - 1;
             int characterNumberZeroBased = characterNumberOneBased - 1;