KEMBAR78
Remove InterceptableAttribute by RikkiGibson · Pull Request #68213 · dotnet/roslyn · GitHub
Skip to content

Remove InterceptableAttribute #68213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
May 17, 2023
Merged

Remove InterceptableAttribute #68213

merged 4 commits into from
May 17, 2023

Conversation

@RikkiGibson
Copy link
Member

@RikkiGibson RikkiGibson commented May 16, 2023 •
edited
Loading

Test plan: #67421
Removing InterceptableAttribute per discussion in #67970.

5cd9e59 and 88bd1a0 are mechanical changes so should be more skimmable.

@ghost ghost added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels May 16, 2023
RikkiGibson
RikkiGibson commented May 16, 2023
View reviewed changes
src/Compilers/CSharp/Test/Semantic/Semantics/InterceptorsTests.cs
public void FileLocalAttributeDefinitions_02()
{
var source0 = """
var source1 = """
Copy link
Member Author

@RikkiGibson RikkiGibson May 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is testing something different than before the PR. The scenario just came to my mind while working on it and rather than moving to test plan and doing it later I decided to get it done now.

@RikkiGibson RikkiGibson removed the untriaged Issues and PRs which have not yet been triaged by a lead label May 16, 2023
@RikkiGibson RikkiGibson marked this pull request as ready for review May 16, 2023 22:44
@RikkiGibson RikkiGibson requested review from a team as code owners May 16, 2023 22:44
@jcouv
Copy link
Member

jcouv commented May 17, 2023 •
edited
Loading

The location of the call is the location of the simple name syntax which denotes the interceptable method. For example, in app.MapGet(...), the name syntax for MapGet would be considered the location of the call. For a static method call like System.Console.WriteLine(...), the name syntax for WriteLine is the location of the call. If we allow intercepting calls to property accessors in the future (e.g obj.Property), we would also be able to use the name syntax in this way.

What does "interceptable method" mean now? (if we decide to remove usages of that term, there's a few more below)

Refers to: docs/features/interceptors.md:82 in ac5cb24. [](commit_id = ac5cb24, deletion_comment = False)

@jcouv jcouv self-assigned this May 17, 2023
jcouv
jcouv approved these changes May 17, 2023
View reviewed changes
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 4) modulo a spec/doc question

@RikkiGibson
Copy link
Member Author

I think we just want distinctive terms for "the method that gets intercepted" and "the method that is intercepting".

@RikkiGibson RikkiGibson merged commit c3465be into dotnet:features/interceptors May 17, 2023
@RikkiGibson RikkiGibson mentioned this pull request Jun 6, 2023
Closed
@RikkiGibson RikkiGibson deleted the ic-4 branch June 6, 2023 22:41
@allisonchou allisonchou mentioned this pull request Jun 16, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jcouv jcouv jcouv approved these changes

+1 more reviewer

@cston cston cston approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jcouv jcouv

Labels

Area-Compilers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@RikkiGibson @jcouv @cston