Interceptors: address duplicate interceptions feedback, handle nameof, add test coverage #67971
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
RikkiGibson
changed the title
src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs
Outdated
Show resolved
Hide resolved
jcouv
removed
the
untriaged
| if (node is BoundNameOfOperator nameofOperator) | ||
| { | ||
| Debug.Assert(!nameofOperator.WasCompilerGenerated); | ||
| var nameofIdentiferSyntax = (IdentifierNameSyntax)((InvocationExpressionSyntax)nameofOperator.Syntax).Expression; |
There was a problem hiding this comment.
I think so, but if this isn't an identifier, I'd like to fail fast here.
| var matchingTrees = syntaxTrees.WhereAsArray(static (tree, filePath) => tree.FilePath == filePath, filePath); | ||
| if (matchingTrees is []) | ||
| SyntaxTree? matchingTree = null; | ||
| foreach (var tree in syntaxTrees) |
There was a problem hiding this comment.
It seems expensive to iterate through the SyntaxTrees collection once for each InterceptsLocationAttribute. Can we avoid this without too much effort?
For instance, it looks like we're already building a HashSet<string> of file paths in CSharpCompilation.CheckDuplicateFilePaths(), at least temporarily. Could we create a Dictionary<string, SyntaxTree> instead, and share between this use and CheckDuplicateFilePaths()?
There was a problem hiding this comment.
Makes total sense. We are also gonna have to make a change here to resolve the FilePath before comparing (i.e. handle /pathmap). Do you mind if I leave a comment for it and resolve in future.
Test plan: #67421
nameof()operator[InterceptsLocation]file path