-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Do not simplify type name to a type with the EditorBrowsable attribute on it. #75235
Do not simplify type name to a type with the EditorBrowsable attribute on it. #75235
Conversation
switch (expression.Kind()) | ||
// Ensure that replacement doesn't change semantics. | ||
var speculationAnalyzer = new SpeculationAnalyzer(expression, replacementNode, semanticModel, cancellationToken); | ||
return !speculationAnalyzer.ReplacementChangesSemantics(); |
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.
removed helper method and pulled this to common location.
} | ||
|
||
enclosingSymbol = enclosingSymbol.ContainingSymbol; | ||
} |
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.
this entire while loop did absolutely nothing.
@@ -40,7 +40,7 @@ public override bool TrySimplify( | |||
replacementNode = null; | |||
issueSpan = default; | |||
|
|||
if (expression is MemberAccessExpressionSyntax { Expression.RawKind: (int)SyntaxKind.ThisExpression } memberAccessExpression) | |||
if (expression is MemberAccessExpressionSyntax(SyntaxKind.ThisExpression) memberAccessExpression) |
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.
simpler pattern form, without unnecessary casts.
|
||
if (TrySimplifyMemberAccessOrQualifiedName(memberAccess.Expression, memberAccess.Name, semanticModel, out var newLeft, out issueSpan)) | ||
switch (expression) |
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.
view with whitespace off.
// replacement node might not be in it's simplest form, so add simplify annotation to it. | ||
replacementNode = memberAccess.Update(newLeft, memberAccess.OperatorToken, memberAccess.Name) | ||
.WithAdditionalAnnotations(Simplifier.Annotation); | ||
|
||
// Ensure that replacement doesn't change semantics. | ||
return !ReplacementChangesSemantics(memberAccess, replacementNode, semanticModel); |
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.
the two calls to ReplacementChangesSemantics were pulled up and then inlined.
// Don't simplify to a base type if it has the EditorBrowsable attribute on it. This is | ||
// occasionally done in some APIs to have a 'pseudo internal' base type that has functionality, | ||
// which a user is supposed to only access through some 'pseudo public' derived type instead. | ||
if (!containingType.IsEditorBrowsable(hideAdvancedMembers: true, semanticModel.Compilation, includingSourceSymbols: true)) |
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.
new logic.
…IDE (#9310) Context: #8381 Add a `DiagnosticSuppressor` to "turn off" the `IDE0002` style diagnostic message which incorrectly tells users to use the `_Microsoft.Android.Resource.Designer.ResourceConstant` type directly. This can cause allot of annoyance with our users because it appears on EVERY single usage of `Resource.*`. So you end up with what looks like code spagetti. So we need to start shipping an `Analyzer` assembly along with the `Ref` framework pack. This is the place these things need to go. Unfortunately it means that the older frameworks will not get this analyzer. Only the current one. On the packaging side, the Analyzer assembly has to go in a `analyzers/dotnet/<language>` folder in the .Ref Nuget Package. There also needs to be an entry in the `FrameworkList.xml` file which has a `Type="Analyzer" ` and a `Language="cs"`. This allows the IDE's to pickup the code. We can ship both regular Analyzers and the DiagnosticSuppressors in the same assembly. So we can extend this with more if needed. How this works is we use Rosyln to look for the IDE0002 diagnsotic message, we then check the code and see if it is refering to a `_Microsoft.Android.Resource.Designer.*` derived class. If it is , we add a suppression. This will stop the hint appearing in the IDE. I have tested this on VS in devbox and it appears to work . Also we generate a Resource Designer assembly and an Intermediate source file at build time. Both of these contain classes which should have the `GeneratedCode` Attribute. So lets add it. The version will be the same as the build assembly used to generate it.
Fixes #75162