Not offering UseAutoProperty for volatile fields & when field used with ref#25431
Not offering UseAutoProperty for volatile fields & when field used with ref#25431DustinCampbell merged 10 commits intodotnet:masterfrom
Conversation
| [Export] | ||
| [DiagnosticAnalyzer(LanguageNames.CSharp)] | ||
| internal class CSharpUseAutoPropertyAnalyzer : AbstractUseAutoPropertyAnalyzer<PropertyDeclarationSyntax, FieldDeclarationSyntax, VariableDeclaratorSyntax, ExpressionSyntax> | ||
| internal class CSharpUseAutoPropertyAnalyzer : AbstractUseAutoPropertyAnalyzer< |
There was a problem hiding this comment.
I only wrapped what went past the GitHub limit (130 characters)
| private static void AddIneligibleField(ISymbol symbol, HashSet<IFieldSymbol> ineligibleFields) | ||
| { | ||
| if (symbol is IFieldSymbol field) | ||
| void AddSymbol(ISymbol symbol) |
There was a problem hiding this comment.
can you still call this "AddIneligibleField"
There was a problem hiding this comment.
@CyrusNajmabadi I originally did but then realized it might be a little misleading because it could add more than 1 symbol
There was a problem hiding this comment.
whereas the previous method called 'AddIneligibleField' only took the symbol (I changed that one into a local function, AddSymbol)
There was a problem hiding this comment.
but I'm not saying it's a good name as is...
There was a problem hiding this comment.
@CyrusNajmabadi wait, I didn't look properly that you pointed this at the local function itself... yes I could call this AddIneligibleField, but what about AddIneligibleFields then? did you notice both of them? should they have names differing in 1 letter only? I figured that it's ok for this to have a short name because it's a small 3 line local function inside a 5 line method
There was a problem hiding this comment.
should they have names differing in 1 letter only?
That's fine. It's fairly common in our code base. A higher level functoin that operates on a collection of items, which calls a helper which works on a single element.
There was a problem hiding this comment.
yes I could call this AddIneligibleField
great.
|
@dotnet-bot retest windows_debug_vs-integration_prtest please |
|
@dotnet-bot retest windows_release_vs-integration_prtest please |
|
retest perf_correctness_prtest please |
|
@dotnet-bot retest this please |
|
retest this please |
|
retest windows_debug_unit32_prtest please |
|
Thanks again @Neme12 |
fixes #25379
fixes #25429