Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Turn on argument exception analyzer on runtime, fix warnings found #38578

Merged
merged 5 commits into from
Jul 2, 2020
Merged

Turn on argument exception analyzer on runtime, fix warnings found #38578

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
900e3ab
Turn on argument exception analyzer on runtime, fix failures found
buyaa-n Jun 29, 2020
ec7228b
Fix warnings on linux build
buyaa-n Jun 29, 2020
d2bd508
Merge branch 'master' of https://github.com/dotnet/runtime into turn_…
buyaa-n Jul 1, 2020
61c8827
Throw same exceptions in Windows as in unix, fix unit tests
buyaa-n Jul 1, 2020
21c3cf6
Fix netFx tests
buyaa-n Jul 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .editorconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ csharp_space_between_square_brackets = false

# Analyzers
dotnet_code_quality.ca1802.api_surface = private, internal
dotnet_code_quality.CA2208.api_surface = public
dotnet_code_quality.ca2208.api_surface = public

# C++ Files
[*.{cpp,h,in}]
Expand Down
2 changes: 1 addition & 1 deletion eng/CodeAnalysis.ruleset
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@
<Rule Id="CA2200" Action="Warning" /> <!-- Rethrow to preserve stack details. -->
<Rule Id="CA2201" Action="None" /> <!-- Do not raise reserved exception types -->
<Rule Id="CA2207" Action="Warning" /> <!-- Initialize value type static fields inline -->
<Rule Id="CA2208" Action="Info" /> <!-- Instantiate argument exceptions correctly -->
<Rule Id="CA2208" Action="Warning" /> <!-- Instantiate argument exceptions correctly -->
<Rule Id="CA2211" Action="None" /> <!-- Non-constant fields should not be visible -->
<Rule Id="CA2213" Action="None" /> <!-- Disposable fields should be disposed -->
<Rule Id="CA2214" Action="None" /> <!-- Do not call overridable methods in constructors -->
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/src/System.Private.CoreLib/src/System/Reflection/Emit/CustomAttributeBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,12 @@ public CustomAttributeBuilder(ConstructorInfo con, object?[] constructorArgs, Pr
throw new ArgumentNullException(nameof(namedFields));
if (fieldValues == null)
throw new ArgumentNullException(nameof(fieldValues));
#pragma warning disable CA2208 // Instantiate argument exceptions correctly, combination of arguments used
if (namedProperties.Length != propertyValues.Length)
throw new ArgumentException(SR.Arg_ArrayLengthsDiffer, "namedProperties, propertyValues");
if (namedFields.Length != fieldValues.Length)
throw new ArgumentException(SR.Arg_ArrayLengthsDiffer, "namedFields, fieldValues");
#pragma warning restore CA2208
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm, we're happy with these argument exception names?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

combination of arguments found 2-3 times in previous PR, and we have suppressed it. I would say we are OK with it instead of happy 😄. We could change the analyzer to not warn in this case but not sure the amount of work for calculating that worth for this rare case scenario


if ((con.Attributes & MethodAttributes.Static) == MethodAttributes.Static ||
(con.Attributes & MethodAttributes.MemberAccessMask) == MethodAttributes.Private)
Expand Down