Skip to content

Conversation

@sibber5
Copy link

@sibber5 sibber5 commented Jul 10, 2025

I didn't change any behavior or any implementation of anything, or any of the overall architecture, just some cleanup

@sibber5
Copy link
Author

sibber5 commented Jul 10, 2025

we could also add polysharp for polyfills for things like init properties

@sibber5
Copy link
Author

sibber5 commented Jul 10, 2025

or maybe bump the tfm to netstandard2.1?

@sibber5
Copy link
Author

sibber5 commented Jul 10, 2025

also you have inconsistent line endings, thats why there are some files that are diffed compeltely. you can copy those to like vscode to see the actual diffs edit: i ended up reusing the original line endings for a cleaner diff.

adding a .gitattributes will handle line endings repo wide, so that this doesnt happen again (see github docs).

should normalize line endings in the future, with .gitattributes
@sibber5 sibber5 changed the title Fix null warnings clean up code Fix null warnings clean up code in CheckedExceptions Jul 11, 2025
@sibber5 sibber5 changed the title Fix null warnings clean up code in CheckedExceptions Fix null warnings and clean up code in CheckedExceptions Jul 11, 2025
@marinasundstrom
Copy link
Owner

Sorry for not seeing the PR sooner. I will have a look at it.

@sibber5
Copy link
Author

sibber5 commented Jul 23, 2025

all good. no pressure!

break;

case AnonymousFunctionExpressionSyntax anonymousFunction:
symbol = context.SemanticModel.GetSymbolInfo(anonymousFunction).Symbol as IMethodSymbol;
Copy link
Author

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?

Copy link
Owner

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);

Copy link
Author

@sibber5 sibber5 Jul 28, 2025

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:

GetSemanticModel is 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 shared SemanticModel, such as RegisterOperationAction, RegisterSyntaxNodeAction, or RegisterSemanticModelAction.

ctrl + shift + f shows this in 3 places, you might want to fix those

Copy link
Owner

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.

@marinasundstrom
Copy link
Owner

Thanks! Great job.

I was thinking that some rules could be enforceable in .editorconfig. Make static fields readonly when possible. Like marking classes as sealed when not extended by others. Etc.

@marinasundstrom marinasundstrom force-pushed the main branch 4 times, most recently from 85b69ca to 81c80de Compare August 5, 2025 12:57
@marinasundstrom marinasundstrom force-pushed the main branch 2 times, most recently from 2d4ad53 to ec75962 Compare August 23, 2025 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants