Skip to content

Remove more delegates from FindRefs.#62027

Merged
CyrusNajmabadi merged 15 commits intodotnet:mainfrom
CyrusNajmabadi:findRefsDelegateRemoval
Jun 21, 2022
Merged

Remove more delegates from FindRefs.#62027
CyrusNajmabadi merged 15 commits intodotnet:mainfrom
CyrusNajmabadi:findRefsDelegateRemoval

Conversation

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

No description provided.

@CyrusNajmabadi CyrusNajmabadi requested a review from dibarbet June 20, 2022 20:42
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner June 20, 2022 20:42

return default;
};
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

both of these static static lambdas just became ordinary methods.

public abstract ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
ISymbol symbol, FindReferencesDocumentState state, FindReferencesSearchOptions options, CancellationToken cancellationToken);

protected virtual ValueTask<(bool matched, CandidateReason reason)> SymbolsMatchAsync(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

these are moves up from the static lambdas we had into normal methods.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

note: This is a virtual as there's one finder currently that overrides it (though i'd like to get rid of htat asap).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

because it's virtual, it became an instance method, so a lot of statics become instance methods

return SymbolsMatchAsync(symbol, state, parent, cancellationToken);
}

protected static async ValueTask<(bool matched, CandidateReason reason)> SymbolsMatchAsync(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

renamed as this is the same as the above method, just taking a node instead of a token.

=> FindDocumentsWithPredicateAsync(project, documents, static (sti, _) => sti.ContainsAwait, /*unused*/false, cancellationToken);

protected static Task<ImmutableArray<Document>> FindDocumentsWithImplicitObjectCreationExpressionAsync(Project project, IImmutableSet<Document>? documents, CancellationToken cancellationToken)
=> FindDocumentsWithPredicateAsync(project, documents, static (sti, _) => sti.ContainsImplicitObjectCreation, /*unused*/false, cancellationToken);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

these were all only referenced from single specific subtypes. so they moved to those subtypes.

cancellationToken);
}

private static Func<ISymbol, FindReferencesDocumentState, SyntaxToken, CancellationToken, ValueTask<(bool matched, CandidateReason reason)>> GetParameterSymbolsMatchFunction(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this got substantively changed. instead of having a single parameter trying to match against lots of different symbols, instead we cascade first to get the different symbols, and then do matching normally.

state,
static (state, token, predefinedType, _) => IsPotentialReference(predefinedType, state.SyntaxFacts, token),
static (_, state, token, _) => ValueTaskFactory.FromResult((matched: true, reason: CandidateReason.None)),
// static (_, state, token, _) => ValueTaskFactory.FromResult((matched: true, reason: CandidateReason.None)),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit - remove

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done.

}

private static Task<ImmutableArray<Document>> FindDocumentsWithImplicitObjectCreationExpressionAsync(Project project, IImmutableSet<Document>? documents, CancellationToken cancellationToken)
=> FindDocumentsWithPredicateAsync(project, documents, static (sti, _) => sti.ContainsImplicitObjectCreation, /*unused*/false, cancellationToken);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

would it be worth having an overload that doesn't take the unused value?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure. let me try that :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks. that does make things nicer!

@ghost ghost added the Area-IDE label Jun 20, 2022
@CyrusNajmabadi CyrusNajmabadi enabled auto-merge June 21, 2022 00:10
@CyrusNajmabadi CyrusNajmabadi merged commit 79d9522 into dotnet:main Jun 21, 2022
@ghost ghost added this to the Next milestone Jun 21, 2022
@CyrusNajmabadi CyrusNajmabadi deleted the findRefsDelegateRemoval branch June 21, 2022 01:54
@RikkiGibson RikkiGibson removed this from the Next milestone Jun 28, 2022
@RikkiGibson RikkiGibson added this to the 17.3 P3 milestone Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants