-
Notifications
You must be signed in to change notification settings - Fork 1.8k
C# 13: Partial properties and indexers. #18533
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
Conversation
ca0f73b
to
ec17a0e
Compare
DCA looks good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 10 out of 25 changed files in this pull request and generated 2 comments.
Files not reviewed (15)
- csharp/ql/test/library-tests/dataflow/fields/FieldFlow.expected: Language not supported
- csharp/ql/test/library-tests/dataflow/indexers/IndexerFlow.expected: Language not supported
- csharp/ql/test/library-tests/dataflow/indexers/IndexerFlow.ql: Language not supported
- csharp/ql/test/library-tests/dispatch/CallGraph.expected: Language not supported
- csharp/ql/test/library-tests/dispatch/GetADynamicTarget.expected: Language not supported
- csharp/ql/test/library-tests/dispatch/viableCallable.expected: Language not supported
- csharp/ql/test/library-tests/partial/MethodIsPartial.expected: Language not supported
- csharp/ql/test/library-tests/partial/Partial1.expected: Language not supported
- csharp/ql/test/library-tests/partial/Partial2.expected: Language not supported
- csharp/ql/test/library-tests/partial/PartialAccessors.expected: Language not supported
- csharp/ql/test/library-tests/partial/PartialAccessors.ql: Language not supported
- csharp/ql/test/library-tests/partial/PartialIndexers.expected: Language not supported
- csharp/extractor/Semmle.Extraction.CSharp/Entities/Indexer.cs: Evaluated as low risk
- csharp/extractor/Semmle.Extraction.CSharp/Entities/Accessor.cs: Evaluated as low risk
- csharp/ql/test/library-tests/dataflow/fields/D.cs: Evaluated as low risk
Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more
… accessor implementations instead of declarations.
1e9a7bd
to
501f985
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, thanks for adding so many tests. The only remark is that we seem to not include all locations for partial properties, like we do for partial methods.
In this PR we make support for partial property and indexer definitions (as described here).
As opposed to partial methods it is worth noting that partial indexers and properties always both have a declaring declaration and an implementation declaration. Thus we need to disregard the declaration get and set (as they are not callables).