-
Notifications
You must be signed in to change notification settings - Fork 231
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
S3655: Static properties and fields called Value #7000
S3655: Static properties and fields called Value #7000
Conversation
79b9328
to
b3538ce
Compare
6da4d84
to
a24b41c
Compare
5c31e36
to
eb18c34
Compare
a24b41c
to
4cf35c7
Compare
class AClassWithStaticValueProperty | ||
{ | ||
public AClassWithInstanceValueProperty InstanceProperty => new AClassWithInstanceValueProperty(); | ||
|
||
public static AClassWithInstanceValueProperty Value => new AClassWithInstanceValueProperty(); | ||
} | ||
|
||
class AClassWithInstanceValueProperty | ||
{ | ||
public AClassWithStaticValueProperty Value => new AClassWithStaticValueProperty(); | ||
} |
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.
These are not value properties.
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.
What is your definition of "value property"? It's a property, and it's named Value
, so I named it ValueProperty.
Would you prefer PropertyNamedValue
instead?
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.
Oh, this makes more sense. I was thinking of value types. I'd go with ClassWithStaticPropertyCalledValue
to avoid confusion.
class AClassWithStaticValueField | ||
{ | ||
public AClassWithInstanceValueField InstanceField = new AClassWithInstanceValueField(); | ||
|
||
public static AClassWithInstanceValueField Value = new AClassWithInstanceValueField(); | ||
} | ||
|
||
class AClassWithInstanceValueField | ||
{ | ||
public AClassWithStaticValueField Value = new AClassWithStaticValueField(); | ||
} |
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.
These are not value fields.
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.
Same as #7000 (comment), I'd say.
b5ba602
to
9480d62
Compare
4cf35c7
to
008e632
Compare
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.
See comments above.
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.
LGTM
} | ||
} | ||
|
||
class AClassWithStaticValueField | ||
class AClassWithStaticFieldCalledValue |
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.
Remove the A
for brevity and consistency.
c9ab91c
to
32b0d1e
Compare
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.
LGTM with some minor polishing
...src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyNullableValueAccessBase.cs
Outdated
Show resolved
Hide resolved
.../tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyNullableValueAccess.cs
Outdated
Show resolved
Hide resolved
.../tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/EmptyNullableValueAccess.cs
Outdated
Show resolved
Hide resolved
d1fa962
to
d6211d9
Compare
8856033
to
74ea7d5
Compare
74ea7d5
to
2dea496
Compare
Kudos, SonarCloud Quality Gate passed! |
Kudos, SonarCloud Quality Gate passed! |
Part 6 of task 6 of #6794
Previous task: #6996
Fixes #6794
Fixes
NullReferenceException
issues encountered on peach, such as: