-
-
Notifications
You must be signed in to change notification settings - Fork 3
Fix null warnings and clean up code in CheckedExceptions #86
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
base: main
Are you sure you want to change the base?
Conversation
|
we could also add polysharp for polyfills for things like |
|
or maybe bump the tfm to netstandard2.1? |
|
also you have inconsistent line endings, adding a |
should normalize line endings in the future, with .gitattributes
|
Sorry for not seeing the PR sooner. I will have a look at it. |
|
all good. no pressure! |
| break; | ||
|
|
||
| case AnonymousFunctionExpressionSyntax anonymousFunction: | ||
| symbol = context.SemanticModel.GetSymbolInfo(anonymousFunction).Symbol as IMethodSymbol; |
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.
@marinasundstrom are you intentionally casting to IMethodSymbol here or did you forget to remove this?
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.
I think that is something left from earlier versions of that method. We certainly don't need it since we assign to a target of type ISymbol.
| continue; | ||
|
|
||
| var semanticModel = context.Compilation.GetSemanticModel(attrSyntax.SyntaxTree); | ||
|
|
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.
var semanticModel = context.Compilation.GetSemanticModel(attrSyntax.SyntaxTree);
@marinasundstrom this gives a warning, see dotnet/roslyn-analyzers#3114:
GetSemanticModelis an expensive method to invoke within a diagnostic analyzer because it creates a completely new semantic model, which does not share compilation data with the compiler or other analyzers. This incurs an additional performance cost during semantic analysis. Instead, consider registering a different analyzer action which allows used of a sharedSemanticModel, such asRegisterOperationAction,RegisterSyntaxNodeAction, orRegisterSemanticModelAction.
ctrl + shift + f shows this in 3 places, you might want to fix those
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.
Thanks for pointing this out. I was honestly not thinking about it. And since I have had help from ChatGPT in many cases, I have just focused on making it work.
Yes, it should be passed down the call tree then.
|
Thanks! Great job. I was thinking that some rules could be enforceable in .editorconfig. Make static fields |
85b69ca to
81c80de
Compare
2d4ad53 to
ec75962
Compare
I didn't change any behavior or any implementation of anything, or any of the overall architecture, just some cleanup