Enable new analyzers CA1510/11/12/13 and CA1856/57/58 #80149
Conversation
|
Tagging subscribers to this area: @dotnet/area-meta Issue DetailsCA1510: Use ArgumentNullException throw helper
|
src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/ContractUtils.cs
Outdated
Show resolved
Hide resolved
stephentoub
force-pushed
the
enableanalyzers
branch
from
422805f to
5419ef1
Compare
|
JFYI: the analyzer found diagnostics on mono builds, other than that LGTM |
stephentoub
requested review from
lewing,
pavelsavara,
thaystg and
radical
as code owners
stephentoub
force-pushed
the
enableanalyzers
branch
from
bb1b442 to
192e1bd
Compare
src/libraries/Common/src/System/Security/Cryptography/EccSecurityTransforms.cs
Show resolved
Hide resolved
CA1510: Use ArgumentNullException throw helper CA1511: Use ArgumentException throw helper CA1512: Use ArgumentOutOfRangeException throw helper CA1513: Use ObjectDisposedException throw helper CA1856: Incorrect usage of ConstantExpected attribute CA1857: A constant is expected for the parameter CA1858: Use 'StartsWith' instead of 'IndexOf'
stephentoub
force-pushed
the
enableanalyzers
branch
from
192e1bd to
95dd85c
Compare
| <!-- Ignore analyzers that recommend APIs introduced in .NET Core when targeting frameworks that lack those APIs | ||
| to avoid issues with multitargeting. | ||
| --> | ||
| <NoWarn Condition="$(TargetFrameworks.Contains('NetFramework')) or $(TargetFrameworks.Contains('netstandard'))">$(NoWarn);CA1510;CA1511;CA1512;CA1513</NoWarn> |
There was a problem hiding this comment.
TargetFrameworks.Contains('NetFramework')) is never true as .NET Framework tfms don't contain such a string in their tfm alias. Instead you want to use TargetFrameworkIdentifier. Using TFI guarantees that the logic works, irrespective of the alias which can be any value.
<NoWarn Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework' or '$(TargetFrameworkIdentifier)' == '.NETStandard'">$(NoWarn);CA1510;CA1511;CA1512;CA1513</NoWarn>
In addition to that, we have projects that don't multi-target and only define a TargetFramework property (singular) which would result in the NoWarn not having effect. By using TFI you can avoid these problems.
There was a problem hiding this comment.
By using TFI you can avoid these problems.
I'm not following. I want these NoWarns to be set when building for .net core if the project also has a net framework or net standard target. How does your proposal achieve that?
There was a problem hiding this comment.
I see, I thought you wanted to set the NoWarn for .NET Framework and .NET Standard builds. Still, $(TargetFrameworks.Contains('NetFramework')) is wrong, we don't have such a TFM. You probably conditioned on the msbuild property name instead of the property value.
We usually use net4 to identify .NET Framework builds in the TargetFrameworks string:
<NoWarn Condition="$(TargetFrameworks.Contains('net4')) or $(TargetFrameworks.Contains('netstandard'))">$(NoWarn);CA1510;CA1511;CA1512;CA1513</NoWarn>
ghost
locked as resolved and limited conversation to collaborators
CA1510: Use ArgumentNullException throw helper
CA1511: Use ArgumentException throw helper
CA1512: Use ArgumentOutOfRangeException throw helper
CA1513: Use ObjectDisposedException throw helper
CA1856: Incorrect usage of ConstantExpected attribute
CA1857: A constant is expected for the parameter
CA1858: Use 'StartsWith' instead of 'IndexOf'