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.

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

C#: Handle some BMN garbage types. #18894

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Feb 28, 2025

In this PR we categorize some types as unknown types even though they are not considered error types in Roslyn in BMN. An example of this is that in the roslyn project a type named var is introduced. Since all files are crammed into a single compilation all implicitly typed declarations are considered to have type var - which is definitely wrong. Instead we should/can consider the type to be unknown (a better solution is of course to get the type right).

Analysis of DCA results:

nightly/nightly: Only roslyn and mono appears to be affected (these are the projects that has different tuple counts on some of the quality queries). This is not surprising as it are those projects that have broken types defined (for instance a type named var and a type without a name). There doesn't appear to be any performance degredations. Tens of thousands of false positives for the cs/call-to-object-tostring have been removed. Some new false positives have been introduced for some of the other quality queries - most prominently is cs/constant-condition. Improving this again will require further investigation. All this boils down to incorrect typing of expressions and variables. DCA also shows an explosion of expressions in roslyn that doesn't have a type. This is expected as most expressions in roslyn were assigned an incorrect type (either the type var or the type without a name).
nightly/security-extended: No changes in results or performance.

@github-actions github-actions bot added the C# label Feb 28, 2025
@michaelnebel michaelnebel force-pushed the csharp/garbagetypes branch 2 times, most recently from 820737e to c4033f2 Compare February 28, 2025 10:19
@@ -267,7 +267,7 @@ private void Populate(ISymbol? optionalSymbol, Entities.CachedEntity entity)

bool duplicationGuard, deferred;

if (ExtractionContext.Mode is ExtractorMode.Standalone)
if (ExtractionContext.IsStandalone)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note, that this is not semantically equivalent! But the original code looks like a mistake.

@michaelnebel michaelnebel force-pushed the csharp/garbagetypes branch 2 times, most recently from d3aa6e2 to 3c5e02f Compare February 28, 2025 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant