-
Notifications
You must be signed in to change notification settings - Fork 245
Fix S6966 FPs/FNs: FPs/FNs after peach validation #9222
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.
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
Changes from 5 commits
3868cc1
9017c58
d92f3fc
1ec24c6
555038a
dda3dd8
7b161eb
a35a929
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,8 +101,9 @@ private static ImmutableArray<ISymbol> FindAwaitableAlternatives(WellKnownExtens | |
| : containingSymbol.ContainingType; // If not dotted, than the scope is the current type. Local function support is missing here. | ||
| var members = GetMethodSymbolsInScope($"{methodSymbol.Name}Async", wellKnownExtensionMethodContainer, invokedType, methodSymbol.ContainingType); | ||
| var awaitableCandidates = members.Where(x => x.IsAwaitableNonDynamic()); | ||
| var awaitableAlternatives = SpeculativeBindCandidates(semanticModel, codeBlock, awaitableRoot, invocationExpression, awaitableCandidates).ToImmutableArray(); | ||
| return awaitableAlternatives; | ||
| var awaitableAlternatives = SpeculativeBindCandidates(semanticModel, codeBlock, awaitableRoot, invocationExpression, awaitableCandidates); | ||
| var withoutContainer = awaitableAlternatives.Where(x => !containingSymbol.Equals(x)).ToImmutableArray(); // Exclude candidates that would resolve to the containing method (endless loop) | ||
| return withoutContainer; | ||
| } | ||
| return ImmutableArray<ISymbol>.Empty; | ||
| } | ||
|
|
@@ -158,7 +159,8 @@ private static IMethodSymbol SpeculativeBindCandidate(SemanticModel semanticMode | |
| } | ||
| if (original == awaitableRoot && result is ExpressionSyntax resultExpression) | ||
|
mary-georgiou marked this conversation as resolved.
|
||
| { | ||
| result = SyntaxFactory.AwaitExpression(resultExpression); | ||
| result = SyntaxFactory.ParenthesizedExpression( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is is possible to add a test that fails if this line is not changed to add parentheses around the replacement?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. This is not possible. This is the funny part with SyntaxTree manipulations: You can end up with a syntax tree that is not expressible in code (If you take the transformed syntax tree, convert it to code, and convert it back to a tree, the new tree looks different). The parentheses are not needed in the tree because the tree looks like parentheses are present. The parentheses will be needed for the code fix though.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe add a quick comment there to capture this?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll merge this to make it to the next release. Let's add a comment in one of the next PRs. |
||
| SyntaxFactory.AwaitExpression(resultExpression.WithoutTrivia().WithLeadingTrivia(SyntaxFactory.ElasticSpace))).WithTriviaFrom(resultExpression); | ||
| } | ||
| return result; | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| using System; | ||
| using System.Linq; | ||
| using System.Threading.Tasks; | ||
| using MongoDB.Driver; | ||
|
|
||
| public record Person(int Id, string Name); | ||
|
|
||
| public class MongoDBDriver | ||
| { | ||
| public async Task Query(IMongoCollection<Person> personCollection) | ||
| { | ||
| var sort = Builders<Person>.Sort.Descending(nameof(Person.Name)); | ||
| // FP for | ||
| // * Find https://mongodb.github.io/mongo-csharp-driver/2.8/apidocs/html/M_MongoDB_Driver_IMongoCollectionExtensions_Find__1_3.htm | ||
| // * FindAsync https://mongodb.github.io/mongo-csharp-driver/2.8/apidocs/html/M_MongoDB_Driver_IMongoCollectionExtensions_FindAsync__1_3.htm | ||
| // Speculative binding finds "FindAsync" but the return type IAsyncCursor<> of FindAsync is not compatible with return type IFindFluent<,> of "Find" | ||
| // Speculative binding does overload resolution according to the C# rules, which ignore return types. | ||
| // It seems to ignore the compiler binding error for the following "Sort" which is only defined on IFindFluent, but not in IAsyncCursor. | ||
| var snapshot = await personCollection.Find(s => s.Id > 10) // Noncompliant FP | ||
| .Sort(sort) // Not defined on IAsyncCursor (return type of FindAsync) | ||
| .FirstOrDefaultAsync().ConfigureAwait(false); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.