Fix "SyntaxTree is not part of the compilation" exception in orchestration analyzers#588
Conversation
Co-authored-by: YunchuWang <12449837+YunchuWang@users.noreply.github.com>
Co-authored-by: YunchuWang <12449837+YunchuWang@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where Roslyn analyzers DURABLE0001-DURABLE0006, DURABLE0009-DURABLE0010 threw ArgumentException: SyntaxTree is not part of the compilation when analyzing orchestration methods that invoke methods from referenced projects.
Key Changes
- Modified
FindInvokedMethods()inMethodProbeOrchestrationVisitorto check if syntax trees are part of the compilation context before attempting to get their semantic models - Changed from using
semanticModel.Compilationtothis.Compilationto access the full compilation context containing all syntax trees being analyzed
| // Since the syntax tree of the callee method might be different from the caller method, we need to get the correct semantic model, | ||
| // avoiding the exception 'Syntax node is not within syntax tree'. | ||
| // We also need to check if the syntax tree is part of the compilation to avoid 'SyntaxTree is not part of the compilation' exception. | ||
| if (!this.Compilation.ContainsSyntaxTree(calleeSyntax.SyntaxTree)) |
There was a problem hiding this comment.
Wouldn't this check only be necessary in the case that semanticModel.SyntaxTree != calleeSyntax.SyntaxTree? If that is the case, shouldn't the ternary assignment below be broken up into an if statement so that this check can be added only in the else case?
There was a problem hiding this comment.
@copilot to check on the above comment (although some human eyes might also be good)
(edit: I guess I'm not allowed to summon the beast...)
There was a problem hiding this comment.
I'll go a step further and suggest that since this check looks expensive, and analyzers are part of a more "performance critical" path, we should probably be sure to only perform it in the else case below where it is needed.
There was a problem hiding this comment.
copilot is not very responsive to mentions in comment threads. you can try mention it at top level
There was a problem hiding this comment.
this contain check is doing a hashlookup of syntaxtree in current set of syntax trees -O(1) perf impact is trivial . it is used in other analyzers frequently https://github.com/dotnet/roslyn/blob/main/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs#L890-L894
| // Since the syntax tree of the callee method might be different from the caller method, we need to get the correct semantic model, | ||
| // avoiding the exception 'Syntax node is not within syntax tree'. | ||
| // We also need to check if the syntax tree is part of the compilation to avoid 'SyntaxTree is not part of the compilation' exception. | ||
| if (!this.Compilation.ContainsSyntaxTree(calleeSyntax.SyntaxTree)) |
There was a problem hiding this comment.
I'll go a step further and suggest that since this check looks expensive, and analyzers are part of a more "performance critical" path, we should probably be sure to only perform it in the else case below where it is needed.
Analyzers threw
ArgumentException: SyntaxTree is not part of the compilationwhen analyzing orchestration methods that invoke methods from referenced projects. Affects DURABLE0001-DURABLE0006, DURABLE0009-DURABLE0010.Root cause
MethodProbeOrchestrationVisitor.FindInvokedMethods()usedsemanticModel.Compilationto get semantic models for invoked methods. When those methods exist in different syntax trees (e.g., referenced projects), the syntax tree isn't part of that compilation instance.Changes
In
OrchestrationAnalyzer.cs, modifiedFindInvokedMethods():this.Compilationinstead ofsemanticModel.Compilationwhen obtaining semantic models for callee syntax treesContainsSyntaxTree()checkthis.Compilationis the full compilation context containing all syntax trees being analyzed, whilesemanticModel.Compilationmay only contain a subset.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.