Skip to content

Fix VSTHRD012 false positive due to inaccessible overloads#1247

Merged
AArnott merged 1 commit into
microsoft:mainfrom
AArnott:fix1111
Oct 24, 2023
Merged

Fix VSTHRD012 false positive due to inaccessible overloads#1247
AArnott merged 1 commit into
microsoft:mainfrom
AArnott:fix1111

Conversation

@AArnott

@AArnott AArnott commented Oct 24, 2023

Copy link
Copy Markdown
Member

Fixes #1111

.Any(m =>
!m.IsObsolete() &&
m.Parameters.Skip(m.IsExtensionMethod ? 1 : 0).Any(IsImportantJtfParameter) &&
context.ContainingSymbol.FindContainingNamedOrAssemblySymbol() is ISymbol containingSymbol && context.Compilation.IsSymbolAccessibleWithin(m, containingSymbol));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

consider splitting each check onto it's own line.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I generally like what you're proposing. This pair in particular share a line because it's really the second check that matters, but the first check is a special check that must be met before the second check can be allowed. All the 'lines' here could be in any order, but are sorted in order of runtime execution cost. So by putting these two checks on the same line, I make it easier to see that they are connected and in a particularly required order.

@AArnott AArnott merged commit c1368fb into microsoft:main Oct 24, 2023
@AArnott AArnott deleted the fix1111 branch October 24, 2023 17:08
@AArnott AArnott added this to the v17.9 milestone Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VSTHRD012 wrongly reported on private/internal members

2 participants