-
Notifications
You must be signed in to change notification settings - Fork 227
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
Migrate S4158: Passing as argument removes constraint #7433
Conversation
{ | ||
"issues": [ | ||
{ | ||
"id": "S4158", | ||
"message": "Remove this call, the collection is known to be empty here.", | ||
"location": { | ||
"uri": "sources\Automapper\src\AutoMapper\Configuration\MappingExpressionBase.cs", | ||
"region": { | ||
"startLine": 174, | ||
"startColumn": 21, | ||
"endLine": 174, | ||
"endColumn": 42 | ||
} | ||
} | ||
}, | ||
{ | ||
"id": "S4158", | ||
"message": "Remove this call, the collection is known to be empty here.", | ||
"location": { | ||
"uri": "sources\Automapper\src\AutoMapper\TypeMap.cs", | ||
"region": { | ||
"startLine": 44, | ||
"startColumn": 17, | ||
"endLine": 44, | ||
"endColumn": 38 | ||
} | ||
} | ||
} | ||
] | ||
} |
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.
FPs
{ | ||
"issues": [ | ||
{ | ||
"id": "S4158", | ||
"message": "Remove this call, the collection is known to be empty here.", | ||
"location": { | ||
"uri": "sources\Automapper\src\AutoMapper\Configuration\MappingExpressionBase.cs", | ||
"region": { | ||
"startLine": 174, | ||
"startColumn": 21, | ||
"endLine": 174, | ||
"endColumn": 42 | ||
} | ||
} | ||
}, | ||
{ | ||
"id": "S4158", | ||
"message": "Remove this call, the collection is known to be empty here.", | ||
"location": { | ||
"uri": "sources\Automapper\src\AutoMapper\TypeMap.cs", | ||
"region": { | ||
"startLine": 44, | ||
"startColumn": 17, | ||
"endLine": 44, | ||
"endColumn": 38 | ||
} | ||
} | ||
} | ||
] | ||
} |
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.
FPs
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.
We'll need to look for arguments to inspect them
...yzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs
Outdated
Show resolved
Hide resolved
1af7576
to
d37f90d
Compare
d219c8e
to
7ec4f7a
Compare
d37f90d
to
789c810
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.
Small ProgramState redesign to make it work better
...yzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs
Show resolved
Hide resolved
7ec4f7a
to
7a4f791
Compare
789c810
to
f106c58
Compare
7a4f791
to
1c14c00
Compare
f106c58
to
91167ce
Compare
1c14c00
to
587cf1a
Compare
91167ce
to
a04b607
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
validator.ValidateTag("AfterField", x => x.Should().BeNull()); | ||
validator.ValidateTag("AfterStaticField", x => x.Should().BeNull()); |
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.
Clean as you code: This method should be refactored to use TagValue
instead. The two will end up as .TagValue("AfterField").Should().HaveNoConstraint()
, effectively ignoring the null
implementation detail (this UT doesn't care)
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 on few methods below - only those you've modified
@@ -200,14 +200,14 @@ public void Test() | |||
validator.TagStates("End").Should().SatisfyRespectively( | |||
x => | |||
{ | |||
x[validator.Symbol("ObjectField")].HasConstraint<ObjectConstraint>().Should().BeFalse(); | |||
x[validator.Symbol("ObjectField")].Should().BeNull(); |
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.
All assertions in this UT should use .Should().HaveNoConstraint()
instead of null, and Should.HaveOnlyConstraints(.....)
for the rest
665175c
to
11ff5ec
Compare
Kudos, SonarCloud Quality Gate passed! |
Kudos, SonarCloud Quality Gate passed! |
Fixes #7300
Draft because it relies on #7418