Turn on argument exception analyzer on runtime, fix warnings found (d…

…otnet#38578) * Turn on argument exception analyzer on runtime, fix failures found
YesReddy1 · Jul 2, 2020 · 06fa941 · 06fa941
1 parent 361065e
commit 06fa941
Show file tree

Hide file tree

Showing 13 changed files with 64 additions and 23 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -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}]

diff --git a/eng/CodeAnalysis.ruleset b/eng/CodeAnalysis.ruleset
@@ -131,7 +131,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 -->

diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/Emit/CustomAttributeBuilder.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/Emit/CustomAttributeBuilder.cs
@@ -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
 
             if ((con.Attributes & MethodAttributes.Static) == MethodAttributes.Static ||
                 (con.Attributes & MethodAttributes.MemberAccessMask) == MethodAttributes.Private)

diff --git a/src/libraries/System.Drawing.Common/src/System/Drawing/Bitmap.Unix.cs b/src/libraries/System.Drawing.Common/src/System/Drawing/Bitmap.Unix.cs
@@ -67,7 +67,7 @@ public Bitmap(Stream stream, bool useIcm)
         public Bitmap(Type type, string resource)
         {
             if (resource == null)
-                throw new ArgumentException(nameof(resource));
+                throw new ArgumentNullException(nameof(resource));
 
             // For compatibility with the .NET Framework
             if (type == null)

diff --git a/src/libraries/System.Drawing.Common/src/System/Drawing/Bitmap.Windows.cs b/src/libraries/System.Drawing.Common/src/System/Drawing/Bitmap.Windows.cs
@@ -12,6 +12,9 @@ public sealed partial class Bitmap
     {
         public Bitmap(Type type, string resource)
         {
+            if (resource == null)
+                throw new ArgumentNullException(nameof(resource));
+
             Stream? stream = type.Module.Assembly.GetManifestResourceStream(type, resource);
             if (stream == null)
             {

diff --git a/src/libraries/System.Drawing.Common/src/System/Drawing/Drawing2D/GraphicsPath.Unix.cs b/src/libraries/System.Drawing.Common/src/System/Drawing/Drawing2D/GraphicsPath.Unix.cs
@@ -381,7 +381,7 @@ public void AddLines(Point[] points)
             if (points == null)
                 throw new ArgumentNullException(nameof(points));
             if (points.Length == 0)
-                throw new ArgumentException(nameof(points));
+                throw new ArgumentException(null, nameof(points));
 
             int status = Gdip.GdipAddPathLine2I(_nativePath, points, points.Length);
             Gdip.CheckStatus(status);
@@ -392,7 +392,7 @@ public void AddLines(PointF[] points)
             if (points == null)
                 throw new ArgumentNullException(nameof(points));
             if (points.Length == 0)
-                throw new ArgumentException(nameof(points));
+                throw new ArgumentException(null, nameof(points));
 
             int status = Gdip.GdipAddPathLine2(_nativePath, points, points.Length);
             Gdip.CheckStatus(status);
@@ -464,7 +464,7 @@ public void AddRectangles(Rectangle[] rects)
             if (rects == null)
                 throw new ArgumentNullException(nameof(rects));
             if (rects.Length == 0)
-                throw new ArgumentException(nameof(rects));
+                throw new ArgumentException(null, nameof(rects));
 
             int status = Gdip.GdipAddPathRectanglesI(_nativePath, rects, rects.Length);
             Gdip.CheckStatus(status);
@@ -475,7 +475,7 @@ public void AddRectangles(RectangleF[] rects)
             if (rects == null)
                 throw new ArgumentNullException(nameof(rects));
             if (rects.Length == 0)
-                throw new ArgumentException(nameof(rects));
+                throw new ArgumentException(null, nameof(rects));
 
             int status = Gdip.GdipAddPathRectangles(_nativePath, rects, rects.Length);
             Gdip.CheckStatus(status);
@@ -642,7 +642,7 @@ public void AddString(string s, FontFamily family, int style, float emSize, Poin
         public void AddString(string s, FontFamily family, int style, float emSize, Rectangle layoutRect, StringFormat? format)
         {
             if (family == null)
-                throw new ArgumentException(nameof(family));
+                throw new ArgumentNullException(nameof(family));
 
             IntPtr sformat = (format == null) ? IntPtr.Zero : format.nativeFormat;
             // note: the NullReferenceException on s.Length is the expected (MS) exception
@@ -653,7 +653,7 @@ public void AddString(string s, FontFamily family, int style, float emSize, Rect
         public void AddString(string s, FontFamily family, int style, float emSize, RectangleF layoutRect, StringFormat? format)
         {
             if (family == null)
-                throw new ArgumentException(nameof(family));
+                throw new ArgumentNullException(nameof(family));
 
             IntPtr sformat = (format == null) ? IntPtr.Zero : format.nativeFormat;
             // note: the NullReferenceException on s.Length is the expected (MS) exception

diff --git a/src/libraries/System.Drawing.Common/src/System/Drawing/Drawing2D/GraphicsPath.Windows.cs b/src/libraries/System.Drawing.Common/src/System/Drawing/Drawing2D/GraphicsPath.Windows.cs
@@ -294,6 +294,8 @@ public unsafe void AddLines(PointF[] points)
         {
             if (points == null)
                 throw new ArgumentNullException(nameof(points));
+            if (points.Length == 0)
+                throw new ArgumentException(null, nameof(points));
 
             fixed (PointF* p = points)
             {
@@ -312,6 +314,8 @@ public unsafe void AddLines(Point[] points)
         {
             if (points == null)
                 throw new ArgumentNullException(nameof(points));
+            if (points.Length == 0)
+                throw new ArgumentException(null, nameof(points));
 
             fixed (Point* p = points)
             {
@@ -526,6 +530,8 @@ public unsafe void AddRectangles(RectangleF[] rects)
         {
             if (rects == null)
                 throw new ArgumentNullException(nameof(rects));
+            if (rects.Length == 0)
+                throw new ArgumentException(null, nameof(rects));
 
             fixed (RectangleF* r = rects)
             {
@@ -545,6 +551,8 @@ public unsafe void AddRectangles(Rectangle[] rects)
         {
             if (rects == null)
                 throw new ArgumentNullException(nameof(rects));
+            if (rects.Length == 0)
+                throw new ArgumentException(null, nameof(rects));
 
             fixed (Rectangle* r = rects)
             {
@@ -639,6 +647,9 @@ public void AddString(string s, FontFamily family, int style, float emSize, Poin
 
         public void AddString(string s, FontFamily family, int style, float emSize, RectangleF layoutRect, StringFormat? format)
         {
+            if (family == null)
+                throw new ArgumentNullException(nameof(family));
+
             Gdip.CheckStatus(Gdip.GdipAddPathString(
                 new HandleRef(this, _nativePath),
                 s,
@@ -652,6 +663,9 @@ public void AddString(string s, FontFamily family, int style, float emSize, Rect
 
         public void AddString(string s, FontFamily family, int style, float emSize, Rectangle layoutRect, StringFormat? format)
         {
+            if (family == null)
+                throw new ArgumentNullException(nameof(family));
+
             Gdip.CheckStatus(Gdip.GdipAddPathStringI(
                 new HandleRef(this, _nativePath),
                 s,

diff --git a/src/libraries/System.Drawing.Common/src/System/Drawing/Icon.Unix.cs b/src/libraries/System.Drawing.Common/src/System/Drawing/Icon.Unix.cs
@@ -205,7 +205,7 @@ public Icon(Icon original, Size size)
                 }
 
                 if (id == ushort.MaxValue)
-                    throw new ArgumentException(SR.NoValidIconImageFound, "Icon");
+                    throw new ArgumentException(SR.NoValidIconImageFound, nameof(original));
 
                 iconSize.Height = iconDir.idEntries[id].height;
                 iconSize.Width = iconDir.idEntries[id].width;
@@ -240,7 +240,7 @@ public Icon(string fileName)
         public Icon(Type type, string resource)
         {
             if (resource == null)
-                throw new ArgumentException(nameof(resource));
+                throw new ArgumentNullException(nameof(resource));
 
             // For compatibility with the .NET Framework
             if (type == null)
@@ -313,7 +313,7 @@ public static Icon ExtractAssociatedIcon(string filePath)
             if (filePath == null)
                 throw new ArgumentNullException(nameof(filePath));
             if (string.IsNullOrEmpty(filePath))
-                throw new ArgumentException(SR.NullOrEmptyPath, "path");
+                throw new ArgumentException(SR.NullOrEmptyPath, nameof(filePath));
             if (!File.Exists(filePath))
                 throw new FileNotFoundException(SR.CouldntFindSpecifiedFile, filePath);
 
@@ -346,7 +346,7 @@ public object Clone()
         public static Icon FromHandle(IntPtr handle)
         {
             if (handle == IntPtr.Zero)
-                throw new ArgumentException(nameof(handle));
+                throw new ArgumentException(null, nameof(handle));
 
             return new Icon(handle);
         }

diff --git a/src/libraries/System.Drawing.Common/src/System/Drawing/Icon.Windows.cs b/src/libraries/System.Drawing.Common/src/System/Drawing/Icon.Windows.cs
@@ -104,6 +104,9 @@ public Icon(Icon original, int width, int height) : this()
 
         public Icon(Type type, string resource) : this()
         {
+            if (resource == null)
+                throw new ArgumentNullException(nameof(resource));
+
             Stream? stream = type.Module.Assembly.GetManifestResourceStream(type, resource);
             if (stream == null)
             {
@@ -167,6 +170,9 @@ void ISerializable.GetObjectData(SerializationInfo si, StreamingContext context)
                 throw new ArgumentNullException(nameof(filePath));
             }
 
+            if (string.IsNullOrEmpty(filePath))
+                throw new ArgumentException(SR.NullOrEmptyPath, nameof(filePath));
+
             filePath = Path.GetFullPath(filePath);
             if (!File.Exists(filePath))
             {
@@ -440,7 +446,13 @@ internal void DrawUnstretched(Graphics graphics, Rectangle targetRect)
 
         ~Icon() => Dispose(false);
 
-        public static Icon FromHandle(IntPtr handle) => new Icon(handle);
+        public static Icon FromHandle(IntPtr handle)
+        {
+            if (handle == IntPtr.Zero)
+                throw new ArgumentException(null, nameof(handle));
+
+            return new Icon(handle);
+        }
 
         // Initializes this Image object.  This is identical to calling the image's
         // constructor with picture, but this allows non-constructor initialization,

diff --git a/src/libraries/System.Drawing.Common/tests/BitmapTests.cs b/src/libraries/System.Drawing.Common/tests/BitmapTests.cs
@@ -111,7 +111,6 @@ public void Ctor_NullType_ThrowsNullReferenceException()
 
         [ActiveIssue("https://github.com/dotnet/runtime/issues/22221", TestPlatforms.AnyUnix)]
         [ConditionalTheory(Helpers.IsDrawingSupported)]
-        [InlineData(typeof(Bitmap), null)]
         [InlineData(typeof(Bitmap), "")]
         [InlineData(typeof(Bitmap), "bitmap_173x183_indexed_8bit.bmp")]
         [InlineData(typeof(BitmapTests), "bitmap_173x183_INDEXED_8bit.bmp")]
@@ -121,6 +120,12 @@ public void Ctor_InvalidResource_ThrowsArgumentException(Type type, string resou
             AssertExtensions.Throws<ArgumentException>(null, () => new Bitmap(type, resource));
         }
 
+        [ConditionalFact(Helpers.IsDrawingSupported)]
+        public void Ctor_InvalidResource_ThrowsArgumentNullException()
+        {
+            AssertExtensions.Throws<ArgumentNullException, ArgumentException>("resource", null, () => new Bitmap(typeof(Bitmap), null));
+        }
+
         [ConditionalTheory(Helpers.IsDrawingSupported)]
         [MemberData(nameof(Ctor_FilePath_TestData))]
         public void Ctor_Stream(string filename, int width, int height, PixelFormat pixelFormat, ImageFormat rawFormat)

diff --git a/src/libraries/System.Drawing.Common/tests/Drawing2D/GraphicsPathTests.cs b/src/libraries/System.Drawing.Common/tests/Drawing2D/GraphicsPathTests.cs
@@ -370,8 +370,8 @@ public void AddLines_PointsNull_ThrowsArgumentNullException()
         [ConditionalFact(Helpers.IsDrawingSupported)]
         public void AddLines_ZeroPoints_ThrowsArgumentException()
         {
-            AssertExtensions.Throws<ArgumentException>(null, () => new GraphicsPath().AddLines(new Point[0]));
-            AssertExtensions.Throws<ArgumentException>(null, () => new GraphicsPath().AddLines(new PointF[0]));
+            AssertExtensions.Throws<ArgumentException>("points", null, () => new GraphicsPath().AddLines(new Point[0]));
+            AssertExtensions.Throws<ArgumentException>("points", null, () => new GraphicsPath().AddLines(new PointF[0]));
         }
 
         [ActiveIssue("https://github.com/dotnet/runtime/issues/22221", TestPlatforms.AnyUnix)]
@@ -1262,11 +1262,11 @@ public void AddString_StringNull_ThrowsNullReferenceException()
         }
 
         [ConditionalFact(Helpers.IsDrawingSupported)]
-        public void AddString_FontFamilyNull_ThrowsArgumentException()
+        public void AddString_FontFamilyNull_ThrowsArgumentNullException()
         {
             using (GraphicsPath gp = new GraphicsPath())
             {
-                AssertExtensions.Throws<ArgumentException>(null, () =>
+                AssertExtensions.Throws<ArgumentNullException, ArgumentException>("family", null, () =>
                     new GraphicsPath().AddString("mono", null, 0, 10, new Point(10, 10), StringFormat.GenericDefault));
             }
         }

diff --git a/src/libraries/System.Drawing.Common/tests/IconTests.cs b/src/libraries/System.Drawing.Common/tests/IconTests.cs
@@ -283,7 +283,6 @@ public void Ctor_NullType_ThrowsNullReferenceException()
         }
 
         [ConditionalTheory(Helpers.IsDrawingSupported)]
-        [InlineData(typeof(Icon), null)]
         [InlineData(typeof(Icon), "")]
         [InlineData(typeof(Icon), "48x48_multiple_entries_4bit.ico")]
         [InlineData(typeof(IconTests), "48x48_MULTIPLE_entries_4bit.ico")]
@@ -292,6 +291,12 @@ public void Ctor_InvalidResource_ThrowsArgumentException(Type type, string resou
             AssertExtensions.Throws<ArgumentException>(null, () => new Icon(type, resource));
         }
 
+        [ConditionalFact(Helpers.IsDrawingSupported)]
+        public void Ctor_InvalidResource_ThrowsArgumentNullException()
+        {
+            AssertExtensions.Throws<ArgumentNullException, ArgumentException>("resource", null, () => new Icon(typeof(Icon), null));
+        }
+
         [ConditionalFact(Helpers.IsDrawingSupported)]
         public void Clone_ConstructedIcon_Success()
         {
@@ -437,7 +442,7 @@ public void ExtractAssociatedIcon_NullFilePath_ThrowsArgumentNullException()
         [ConditionalFact(Helpers.IsDrawingSupported)]
         public void ExtractAssociatedIcon_InvalidFilePath_ThrowsArgumentException()
         {
-            AssertExtensions.Throws<ArgumentException>("path", null, () => Icon.ExtractAssociatedIcon(""));
+            AssertExtensions.Throws<ArgumentException>("filePath", null, () => Icon.ExtractAssociatedIcon(""));
         }
 
 
@@ -759,7 +764,7 @@ public void FromHandle_BitmapHandleMultipleTime_Success()
         [ConditionalFact(Helpers.IsDrawingSupported)]
         public void FromHandle_Zero_ThrowsArgumentException()
         {
-            AssertExtensions.Throws<ArgumentException>(null, () => Icon.FromHandle(IntPtr.Zero));
+            AssertExtensions.Throws<ArgumentException>("handle", null, () => Icon.FromHandle(IntPtr.Zero));
         }
 
         [ConditionalFact(Helpers.IsDrawingSupported)]

diff --git a/src/libraries/System.Drawing.Common/tests/mono/System.Drawing/BitmapTests.cs b/src/libraries/System.Drawing.Common/tests/mono/System.Drawing/BitmapTests.cs
@@ -1304,7 +1304,7 @@ public void BitmapTypeStringCtor1()
         [ConditionalFact(Helpers.IsDrawingSupported)]
         public void BitmapTypeStringCtor2()
         {
-            Assert.Throws<ArgumentException>(() => new Bitmap(typeof(Bitmap), null));
+            AssertExtensions.Throws<ArgumentNullException, ArgumentException>("resource", null, () => new Bitmap(typeof(Bitmap), null));
         }
 
         private void SetResolution(float x, float y)