Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/Analyzers/Orchestration/OrchestrationAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,15 @@ void FindInvokedMethods(
{
// 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))
Copy link

@alainbryden alainbryden Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link

@alainbryden alainbryden Jan 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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...)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copilot is not very responsive to mentions in comment threads. you can try mention it at top level

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

{
// Skip this syntax tree if it's not part of the current compilation context (e.g., from referenced projects or source generators)
continue;
}

SemanticModel sm = semanticModel.SyntaxTree == calleeSyntax.SyntaxTree ?
semanticModel : semanticModel.Compilation.GetSemanticModel(calleeSyntax.SyntaxTree);
semanticModel : this.Compilation.GetSemanticModel(calleeSyntax.SyntaxTree);

this.FindInvokedMethods(sm, calleeSyntax, calleeMethodSymbol, rootOrchestration, reportDiagnostic);
}
Expand Down
Loading