Skip to content
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

Location (highlighting) of IL2026 diagnostic could be improved #109823

Open
YoshiRulz opened this issue Nov 14, 2024 · 4 comments · May be fixed by #110086
Open

Location (highlighting) of IL2026 diagnostic could be improved #109823

YoshiRulz opened this issue Nov 14, 2024 · 4 comments · May be fixed by #110086
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers in-pr There is an active PR which will close this issue when it is merged
Milestone
10.0.0

Comments

@YoshiRulz
Copy link 

void Current() => {|IL2026:JsonSerializer.Serialize(obj.LongMethodCallWith(args))|};
void Proposal() => {|IL2026:JsonSerializer.Serialize|}(obj.LongMethodCallWith(args));

Apologies if this is the wrong repo. I believe this is the source for the Analyzer, but it's not obvious where the relevant GetLocation call is.
I have version 8.0.110 of the SDK installed.
Moved from dotnet/roslyn#75914.
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 14, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Nov 14, 2024
@kumariaditi52

This comment has been minimized.

Sign in to view
@huoyaoyuan huoyaoyuan added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 15, 2024
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

@jtschuster
Copy link
Member

@sbomer any thoughts? I think it could be helpful to put the warning just on the method reference, especially in cases where the method spans multiple lines and arguments are method calls. Then these calls wouldn't have overlapping warnings:

MethodWithRUC(
  AnotherMethodWithRuc(),
  MethodWithRuc()
);

@sbomer
Copy link
Member

sbomer commented Nov 21, 2024

Agreed, I like the suggestion!

@sbomer sbomer removed the untriaged New issue has not been triaged by the area owner label Nov 21, 2024
@sbomer sbomer added this to the 10.0.0 milestone Nov 21, 2024
@jtschuster jtschuster linked a pull request Nov 22, 2024 that will close this issue
Open
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers in-pr There is an active PR which will close this issue when it is merged
Projects
AppModel
Status: No status
Milestone
10.0.0
Development

Successfully merging a pull request may close this issue.

Warn on method access in Requires analyzers jtschuster/runtime
5 participants
@sbomer @huoyaoyuan @YoshiRulz @jtschuster @kumariaditi52