FxCop with ColorPicker (#6464)

* fxcop fixes

* more fixes, not done yet

* supressing 1031 and ca2000 since we are expressly disposing this correctly

* catching a possible crash due to null ref if run twice

* addressing feedback
This commit is contained in:
Clint Rutkas 2020-09-09 17:08:37 -07:00 committed by GitHub
parent 1027b7de72
commit cfda69a120
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 137 additions and 54 deletions

View File

@ -14,11 +14,12 @@ namespace ColorPickerUI
/// <summary>
/// Interaction logic for App.xaml
/// </summary>
public partial class App : Application
public partial class App : Application, IDisposable
{
private Mutex _instanceMutex = null;
private Mutex _instanceMutex;
private static string[] _args;
private int _powerToysPid;
private bool disposedValue;
[STAThread]
public static void Main(string[] args)
@ -27,11 +28,15 @@ namespace ColorPickerUI
AppDomain.CurrentDomain.UnhandledException += CurrentDomain_UnhandledException;
try
{
var application = new App();
application.InitializeComponent();
application.Run();
using (var application = new App())
{
application.InitializeComponent();
application.Run();
}
}
#pragma warning disable CA1031 // Do not catch general exception types
catch (Exception ex)
#pragma warning restore CA1031 // Do not catch general exception types
{
Logger.LogError("Unhandled exception", ex);
CursorManager.RestoreOriginalCursors();
@ -78,5 +83,27 @@ namespace ColorPickerUI
Logger.LogError("Unhandled exception", (e.ExceptionObject is Exception) ? (e.ExceptionObject as Exception) : new Exception());
CursorManager.RestoreOriginalCursors();
}
protected virtual void Dispose(bool disposing)
{
if (!disposedValue)
{
if (disposing)
{
_instanceMutex?.Dispose();
}
// TODO: free unmanaged resources (unmanaged objects) and override finalizer
// TODO: set large fields to null
disposedValue = true;
}
}
public void Dispose()
{
// Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
Dispose(disposing: true);
GC.SuppressFinalize(this);
}
}
}

View File

@ -10,12 +10,11 @@ namespace ColorPicker.Behaviors
{
public class GridEffectBehavior : Behavior<FrameworkElement>
{
private static double baseZoomImageSizeInPx = 50;
private static double _baseZoomImageSizeInPx = 50;
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.MaintainabilityRules", "SA1401:Fields should be private", Justification = "https://docs.microsoft.com/en-us/dotnet/framework/wpf/advanced/dependency-property-security#:~:text=Dependency%20properties%20should%20generally%20be%20considered%20to%20be,make%20security%20guarantees%20about%20a%20dependency%20property%20value.")]
public static DependencyProperty EffectProperty = DependencyProperty.Register("Effect", typeof(GridShaderEffect), typeof(GridEffectBehavior));
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.MaintainabilityRules", "SA1401:Fields should be private", Justification = "https://docs.microsoft.com/en-us/dotnet/framework/wpf/advanced/dependency-property-security#:~:text=Dependency%20properties%20should%20generally%20be%20considered%20to%20be,make%20security%20guarantees%20about%20a%20dependency%20property%20value.")]
public static DependencyProperty ZoomFactorProperty = DependencyProperty.Register("ZoomFactor", typeof(double), typeof(GridEffectBehavior));
public static readonly DependencyProperty EffectProperty = DependencyProperty.Register("Effect", typeof(GridShaderEffect), typeof(GridEffectBehavior));
public static readonly DependencyProperty ZoomFactorProperty = DependencyProperty.Register("ZoomFactor", typeof(double), typeof(GridEffectBehavior));
public GridShaderEffect Effect
{
@ -46,11 +45,12 @@ namespace ColorPicker.Behaviors
var relativeX = position.X / AssociatedObject.ActualWidth;
var relativeY = position.Y / AssociatedObject.Height;
Effect.MousePosition = new Point(relativeX, relativeY);
if (ZoomFactor >= 4)
{
Effect.Radius = 0.04;
Effect.SquareSize = ZoomFactor;
Effect.TextureSize = baseZoomImageSizeInPx * ZoomFactor;
Effect.TextureSize = _baseZoomImageSizeInPx * ZoomFactor;
}
else
{

View File

@ -11,8 +11,7 @@ namespace ColorPicker.Behaviors
{
public class MoveWindowBehavior : Behavior<Window>
{
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.MaintainabilityRules", "SA1401:Fields should be private", Justification = "https://docs.microsoft.com/en-us/dotnet/framework/wpf/advanced/dependency-property-security#:~:text=Dependency%20properties%20should%20generally%20be%20considered%20to%20be,make%20security%20guarantees%20about%20a%20dependency%20property%20value.")]
public static DependencyProperty LeftProperty = DependencyProperty.Register("Left", typeof(double), typeof(MoveWindowBehavior), new PropertyMetadata(new PropertyChangedCallback(LeftPropertyChanged)));
public static readonly DependencyProperty LeftProperty = DependencyProperty.Register("Left", typeof(double), typeof(MoveWindowBehavior), new PropertyMetadata(new PropertyChangedCallback(LeftPropertyChanged)));
private static void LeftPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
{
@ -22,8 +21,7 @@ namespace ColorPicker.Behaviors
sender.BeginAnimation(Window.LeftProperty, move, HandoffBehavior.Compose);
}
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.MaintainabilityRules", "SA1401:Fields should be private", Justification = "https://docs.microsoft.com/en-us/dotnet/framework/wpf/advanced/dependency-property-security#:~:text=Dependency%20properties%20should%20generally%20be%20considered%20to%20be,make%20security%20guarantees%20about%20a%20dependency%20property%20value.")]
public static DependencyProperty TopProperty = DependencyProperty.Register("Top", typeof(double), typeof(MoveWindowBehavior), new PropertyMetadata(new PropertyChangedCallback(TopPropertyChanged)));
public static readonly DependencyProperty TopProperty = DependencyProperty.Register("Top", typeof(double), typeof(MoveWindowBehavior), new PropertyMetadata(new PropertyChangedCallback(TopPropertyChanged)));
private static void TopPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
{

View File

@ -11,8 +11,7 @@ namespace ColorPicker.Behaviors
{
public class ResizeBehavior : Behavior<FrameworkElement>
{
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.MaintainabilityRules", "SA1401:Fields should be private", Justification = "https://docs.microsoft.com/en-us/dotnet/framework/wpf/advanced/dependency-property-security#:~:text=Dependency%20properties%20should%20generally%20be%20considered%20to%20be,make%20security%20guarantees%20about%20a%20dependency%20property%20value.")]
public static DependencyProperty WidthProperty = DependencyProperty.Register("Width", typeof(double), typeof(ResizeBehavior), new PropertyMetadata(new PropertyChangedCallback(WidthPropertyChanged)));
public static readonly DependencyProperty WidthProperty = DependencyProperty.Register("Width", typeof(double), typeof(ResizeBehavior), new PropertyMetadata(new PropertyChangedCallback(WidthPropertyChanged)));
private static void WidthPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
{
@ -28,8 +27,7 @@ namespace ColorPicker.Behaviors
sender.BeginAnimation(FrameworkElement.WidthProperty, move, HandoffBehavior.Compose);
}
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.MaintainabilityRules", "SA1401:Fields should be private", Justification = "https://docs.microsoft.com/en-us/dotnet/framework/wpf/advanced/dependency-property-security#:~:text=Dependency%20properties%20should%20generally%20be%20considered%20to%20be,make%20security%20guarantees%20about%20a%20dependency%20property%20value.")]
public static DependencyProperty HeightProperty = DependencyProperty.Register("Height", typeof(double), typeof(ResizeBehavior), new PropertyMetadata(new PropertyChangedCallback(HeightPropertyChanged)));
public static readonly DependencyProperty HeightProperty = DependencyProperty.Register("Height", typeof(double), typeof(ResizeBehavior), new PropertyMetadata(new PropertyChangedCallback(HeightPropertyChanged)));
private static void HeightPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
{

View File

@ -11,6 +11,7 @@ namespace ColorPicker
{
public static CompositionContainer Container { get; private set; }
[System.Diagnostics.CodeAnalysis.SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "This is properly disposed of in MainWindow.Xaml.cs")]
public static void InitializeContainer(object initPoint)
{
var catalog = new AssemblyCatalog(System.Reflection.Assembly.GetExecutingAssembly());

View File

@ -109,6 +109,7 @@
<Compile Include="Behaviors\GridEffectBehavior.cs" />
<Compile Include="Helpers\IThrottledActionInvoker.cs" />
<Compile Include="Helpers\ThrottledActionInvoker.cs" />
<Compile Include="NativeMethods.cs" />
<Compile Include="Settings\IUserSettings.cs" />
<Compile Include="Settings\SettingItem`1.cs" />
<Compile Include="Settings\UserSettings.cs" />
@ -117,7 +118,6 @@
<Compile Include="Telemetry\ColorPickerCancelledEvent.cs" />
<Compile Include="Telemetry\ColorPickerShowEvent.cs" />
<Compile Include="Telemetry\ColorPickerZoomOpenedEvent.cs" />
<Compile Include="Win32Apis.cs" />
<Compile Include="ZoomWindow.xaml.cs">
<DependentUpon>ZoomWindow.xaml</DependentUpon>
</Compile>
@ -216,6 +216,11 @@
<None Include="App.config" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers">
<Version>3.3.0</Version>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
<PackageReference Include="System.Windows.Interactivity.WPF">
<Version>2.0.20525</Version>
</PackageReference>

View File

@ -9,7 +9,7 @@ using System.Linq;
using System.Runtime.InteropServices;
using System.Windows;
using System.Windows.Media;
using static ColorPicker.Win32Apis;
using static ColorPicker.NativeMethods;
namespace ColorPicker.Helpers
{
@ -17,6 +17,7 @@ namespace ColorPicker.Helpers
{
public static readonly HandleRef NullHandleRef = new HandleRef(null, IntPtr.Zero);
[System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA1801:Review unused parameters", Justification = "Interop")]
private MonitorResolutionHelper(IntPtr monitor, IntPtr hdc)
{
var info = new MonitorInfoEx();
@ -73,6 +74,7 @@ namespace ColorPicker.Helpers
public ArrayList Monitors { get; private set; }
[System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA1801:Review unused parameters", Justification = "Interop")]
public bool Callback(
IntPtr monitor,
IntPtr hdc,

View File

@ -28,8 +28,8 @@ namespace ColorPicker.Helpers
private readonly AppStateHandler _appStateHandler;
private readonly IThrottledActionInvoker _throttledActionInvoker;
private int _currentZoomLevel = 0;
private int _previousZoomLevel = 0;
private int _currentZoomLevel;
private int _previousZoomLevel;
private ZoomWindow _zoomWindow;
@ -90,14 +90,17 @@ namespace ColorPicker.Helpers
var x = (int)point.X - (BaseZoomImageSize / 2);
var y = (int)point.Y - (BaseZoomImageSize / 2);
var rect = new Rectangle(x, y, BaseZoomImageSize, BaseZoomImageSize);
var bmp = new Bitmap(rect.Width, rect.Height, PixelFormat.Format32bppArgb);
var g = Graphics.FromImage(bmp);
g.CopyFromScreen(rect.Left, rect.Top, 0, 0, bmp.Size, CopyPixelOperation.SourceCopy);
var bitmapImage = BitmapToImageSource(bmp);
using (var bmp = new Bitmap(rect.Width, rect.Height, PixelFormat.Format32bppArgb))
{
var g = Graphics.FromImage(bmp);
g.CopyFromScreen(rect.Left, rect.Top, 0, 0, bmp.Size, CopyPixelOperation.SourceCopy);
_zoomViewModel.ZoomArea = bitmapImage;
_zoomViewModel.ZoomFactor = 1;
var bitmapImage = BitmapToImageSource(bmp);
_zoomViewModel.ZoomArea = bitmapImage;
_zoomViewModel.ZoomFactor = 1;
}
}
else
{
@ -110,17 +113,19 @@ namespace ColorPicker.Helpers
ShowZoomWindow((int)point.X, (int)point.Y);
}
private BitmapSource BitmapToImageSource(Bitmap bitmap)
private static BitmapSource BitmapToImageSource(Bitmap bitmap)
{
using (MemoryStream memory = new MemoryStream())
{
bitmap.Save(memory, ImageFormat.Bmp);
memory.Position = 0;
BitmapImage bitmapimage = new BitmapImage();
bitmapimage.BeginInit();
bitmapimage.StreamSource = memory;
bitmapimage.CacheOption = BitmapCacheOption.OnLoad;
bitmapimage.EndInit();
return bitmapimage;
}
}

View File

@ -6,7 +6,7 @@ using System;
using System.ComponentModel;
using System.Diagnostics;
using System.Runtime.InteropServices;
using static ColorPicker.Win32Apis;
using static ColorPicker.NativeMethods;
namespace ColorPicker.Keyboard
{

View File

@ -3,7 +3,7 @@
// See the LICENSE file in the project root for more information.
using System.ComponentModel;
using static ColorPicker.Win32Apis;
using static ColorPicker.NativeMethods;
namespace ColorPicker.Keyboard
{

View File

@ -11,18 +11,19 @@ using ColorPicker.Settings;
using ColorPicker.Telemetry;
using Microsoft.PowerToys.Settings.UI.Lib.Utilities;
using Microsoft.PowerToys.Telemetry;
using static ColorPicker.Win32Apis;
using static ColorPicker.NativeMethods;
namespace ColorPicker.Keyboard
{
[Export(typeof(KeyboardMonitor))]
public class KeyboardMonitor
public class KeyboardMonitor : IDisposable
{
private readonly AppStateHandler _appStateHandler;
private readonly IUserSettings _userSettings;
private List<string> _activationKeys = new List<string>();
private GlobalKeyboardHook _keyboardHook;
private bool disposedValue;
[ImportingConstructor]
public KeyboardMonitor(AppStateHandler appStateHandler, IUserSettings userSettings)
@ -135,5 +136,28 @@ namespace ColorPicker.Keyboard
currentlyPressedKeys.Add("Win");
}
}
protected virtual void Dispose(bool disposing)
{
if (!disposedValue)
{
if (disposing)
{
// TODO: dispose managed state (managed objects)
_keyboardHook.Dispose();
}
// TODO: free unmanaged resources (unmanaged objects) and override finalizer
// TODO: set large fields to null
disposedValue = true;
}
}
public void Dispose()
{
// Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
Dispose(disposing: true);
GC.SuppressFinalize(this);
}
}
}

View File

@ -50,14 +50,16 @@ namespace ColorPicker.Mouse
if (curFile != null)
{
Registry.SetValue(CursorsRegistryPath, cursorRegistryName, curFile);
Win32Apis.SystemParametersInfo(SPI_SETCURSORS, 0, new IntPtr(0), SPIF_SENDCHANGE);
NativeMethods.SystemParametersInfo(SPI_SETCURSORS, 0, new IntPtr(0), SPIF_SENDCHANGE);
}
else
{
Logger.LogInfo("Cursor file path was null");
}
}
#pragma warning disable CA1031 // Do not catch general exception types
catch (Exception ex)
#pragma warning restore CA1031 // Do not catch general exception types
{
Logger.LogError("Failed to change cursor", ex);
}

View File

@ -7,7 +7,7 @@ using System.Diagnostics;
using System.Runtime.InteropServices;
using System.Windows.Input;
using ColorPicker.Helpers;
using static ColorPicker.Win32Apis;
using static ColorPicker.NativeMethods;
namespace ColorPicker.Mouse
{
@ -21,8 +21,6 @@ namespace ColorPicker.Mouse
private const int WM_LBUTTONDOWN = 0x0201;
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop object")]
private const int WM_MOUSEWHEEL = 0x020A;
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop object")]
private const int WHEEL_DELTA = 120;
private IntPtr _mouseHookHandle;
private HookProc _mouseDelegate;

View File

@ -10,7 +10,7 @@ using System.Windows.Input;
using System.Windows.Threading;
using ColorPicker.Helpers;
using ColorPicker.Settings;
using static ColorPicker.Win32Apis;
using static ColorPicker.NativeMethods;
namespace ColorPicker.Mouse
{
@ -31,9 +31,13 @@ namespace ColorPicker.Mouse
_timer.Interval = TimeSpan.FromMilliseconds(MousePullInfoIntervalInMs);
_timer.Tick += Timer_Tick;
appStateMonitor.AppShown += AppStateMonitor_AppShown;
appStateMonitor.AppClosed += AppStateMonitor_AppClosed;
appStateMonitor.AppHidden += AppStateMonitor_AppClosed;
if (appStateMonitor != null)
{
appStateMonitor.AppShown += AppStateMonitor_AppShown;
appStateMonitor.AppClosed += AppStateMonitor_AppClosed;
appStateMonitor.AppHidden += AppStateMonitor_AppClosed;
}
_mouseHook = new MouseHook();
_userSettings = userSettings;
}
@ -79,11 +83,13 @@ namespace ColorPicker.Mouse
private static Color GetPixelColor(System.Windows.Point mousePosition)
{
var rect = new Rectangle((int)mousePosition.X, (int)mousePosition.Y, 1, 1);
var bmp = new Bitmap(rect.Width, rect.Height, PixelFormat.Format32bppArgb);
var g = Graphics.FromImage(bmp);
g.CopyFromScreen(rect.Left, rect.Top, 0, 0, bmp.Size, CopyPixelOperation.SourceCopy);
using (var bmp = new Bitmap(rect.Width, rect.Height, PixelFormat.Format32bppArgb))
{
var g = Graphics.FromImage(bmp);
g.CopyFromScreen(rect.Left, rect.Top, 0, 0, bmp.Size, CopyPixelOperation.SourceCopy);
return bmp.GetPixel(0, 0);
return bmp.GetPixel(0, 0);
}
}
private static System.Windows.Point GetCursorPosition()

View File

@ -8,9 +8,12 @@ using System.Runtime.Versioning;
namespace ColorPicker
{
public static class Win32Apis
// https://docs.microsoft.com/en-us/visualstudio/code-quality/ca1060?view=vs-2019
// will have to rename
public static class NativeMethods
{
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop")]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1707:Identifiers should not contain underscores", Justification = "Interop")]
public const int WH_KEYBOARD_LL = 13;
public const int VkSnapshot = 0x2c;
public const int KfAltdown = 0x2000;
@ -18,14 +21,23 @@ namespace ColorPicker
public const int MonitorinfofPrimary = 0x00000001;
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop")]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1707:Identifiers should not contain underscores", Justification = "Interop")]
public const int VK_SHIFT = 0x10;
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop")]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1707:Identifiers should not contain underscores", Justification = "Interop")]
public const int VK_CONTROL = 0x11;
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop")]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1707:Identifiers should not contain underscores", Justification = "Interop")]
public const int VK_MENU = 0x12;
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop")]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1707:Identifiers should not contain underscores", Justification = "Interop")]
public const int VK_LWIN = 0x5B;
[System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop")]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1707:Identifiers should not contain underscores", Justification = "Interop")]
public const int VK_RWIN = 0x5C;
public delegate bool MonitorEnumProc(
@ -114,9 +126,9 @@ namespace ColorPicker
internal class MonitorInfoEx
{
public int cbSize = Marshal.SizeOf(typeof(MonitorInfoEx));
public Rect rcMonitor = default;
public Rect rcWork = default;
public int dwFlags = 0;
public Rect rcMonitor;
public Rect rcWork;
public int dwFlags;
[MarshalAs(UnmanagedType.ByValArray, SizeConst = 32)]
public char[] szDevice = new char[32];
}

View File

@ -83,7 +83,9 @@ namespace ColorPicker.Settings
Logger.LogError("Failed to read changed settings", ex);
Thread.Sleep(500);
}
#pragma warning disable CA1031 // Do not catch general exception types
catch (Exception ex)
#pragma warning restore CA1031 // Do not catch general exception types
{
if (retryCount > MaxNumberOfRetry)
{

View File

@ -42,11 +42,14 @@ namespace ColorPicker.ViewModels
_appStateHandler = appStateHandler;
_userSettings = userSettings;
mouseInfoProvider.MouseColorChanged += Mouse_ColorChanged;
mouseInfoProvider.OnMouseDown += MouseInfoProvider_OnMouseDown;
mouseInfoProvider.OnMouseWheel += MouseInfoProvider_OnMouseWheel;
if (mouseInfoProvider != null)
{
mouseInfoProvider.MouseColorChanged += Mouse_ColorChanged;
mouseInfoProvider.OnMouseDown += MouseInfoProvider_OnMouseDown;
mouseInfoProvider.OnMouseWheel += MouseInfoProvider_OnMouseWheel;
}
keyboardMonitor.Start();
keyboardMonitor?.Start();
}
public string HexColor