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.

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

Adding FxCop to Microsoft.Plugin.WindowWalker #6260

Conversation

ryanbodrug-microsoft
Copy link
Contributor

Summary of the Pull Request

What is this about?

PR Checklist

  • Applies to [Chore] Enable Static analysis (FxCopAnalyzers) on C# Projects #5129
  • CLA signed. If not, go over here and sign the CLA
  • Tests added/passed
  • Requires documentation to be updated
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Info on Pull Request

What does this include?

Fixes the following errors:
Code Description
CA1304 The behavior of 'string.ToLower()' could vary based on the current user's locale settings. Replace this call in 'Window.SwitchToWindow()' with a call to 'string.ToLower(CultureInfo)'.
CA1812 WindowResult is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it static (Shared in Visual Basic).
CA1806 UpdateOpenWindowsList calls EnumWindows but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
CA1066 Type Microsoft.Plugin.WindowWalker.Components.InteropAndHelpers.RECT should implement IEquatable because it overrides Equals
CA1052 Type 'FuzzyMatching' is a static holder type but is neither static nor NotInheritable
CA1069 The enum member 'WS_EX_RIGHTSCROLLBAR' has the same constant value '0' as member 'WS_EX_LEFT'
CA1069 The enum member 'WS_EX_LTRREADING' has the same constant value '0' as member 'WS_EX_LEFT'
CA1069 The enum member 'SWP_NOREPOSITION' has the same constant value '512' as member 'SWP_NOOWNERZORDER'
CA1069 The enum member 'SWP_FRAMECHANGED' has the same constant value '32' as member 'SWP_DRAWFRAME'
CA1069 The enum member 'ShowMaximized' has the same constant value '3' as member 'Maximize'
CA1304 The behavior of 'string.ToUpper()' could vary based on the current user's locale settings. Replace this call in 'Window.ToString()' with a call to 'string.ToUpper(CultureInfo)'.
CA1304 The behavior of 'string.ToLower()' could vary based on the current user's locale settings. Replace this call in 'Window.ProcessName.get' with a call to 'string.ToLower(CultureInfo)'.
CA1304 The behavior of 'string.ToLower()' could vary based on the current user's locale settings. Replace this call in 'SearchController.SearchText.set' with a call to 'string.ToLower(CultureInfo)'.
CA1304 The behavior of 'string.ToLower()' could vary based on the current user's locale settings. Replace this call in 'FuzzyMatching.FindBestFuzzyMatch(string, string)' with a call to 'string.ToLower(CultureInfo)'.
CA1304 The behavior of 'string.ToLower()' could vary based on the current user's locale settings. Replace this call in 'FuzzyMatching.FindBestFuzzyMatch(string, string)' with a call to 'string.ToLower(CultureInfo)'.
CA1307 The behavior of 'string.Equals(string?)' could vary based on the current user's locale settings. Replace this call in 'Microsoft.Plugin.WindowWalker.Components.Window.SwitchToWindow()' with a call to 'string.Equals(string?, System.StringComparison)'.
CA1820 Test for empty strings using 'string.Length' property or 'string.IsNullOrEmpty' method instead of an Equality check
CA2101 Specify marshaling for P/Invoke string arguments
CA2101 Specify marshaling for P/Invoke string arguments
CA2101 Specify marshaling for P/Invoke string arguments
CA1806 SetWindowExclusionFromLivePreview calls DwmSetWindowAttribute but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
CA1710 Rename SearchResultUpdateHandler to end in 'EventHandler'
CA1710 Rename OpenWindowsUpdateHandler to end in 'EventHandler'
CA1801 Parameter value of method remove_OnOpenWindowsUpdate is never used. Remove the parameter or use it in the method body.
CA1801 Parameter value of method add_OnOpenWindowsUpdate is never used. Remove the parameter or use it in the method body.
CA1060 Move pinvokes to native methods class
CA1031 Modify 'get_WindowIcon' to catch a more specific allowed exception type, or rethrow the exception
CA1822 Member 'OnOpenWindowsUpdate' does not access instance data and can be marked as static
CA1822 Member 'GetProcessIDFromWindowHandle' does not access instance data and can be marked as static
CA1814 matches is a multidimensional array. Replace it with a jagged array if possible.
CA1806 IsWindowCloaked calls DwmGetWindowAttribute but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
CA1062 In externally visible method 'List Main.Query(Query query)', validate parameter 'query' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.
CA1062 In externally visible method 'List<List> FuzzyMatching.GetAllMatchIndexes(bool[,] matches)', validate parameter 'matches' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.
CA1062 In externally visible method 'List FuzzyMatching.FindBestFuzzyMatch(string text, string searchText)', validate parameter 'text' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.
CA1062 In externally visible method 'List FuzzyMatching.FindBestFuzzyMatch(string text, string searchText)', validate parameter 'searchText' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.
CA1062 In externally visible method 'int FuzzyMatching.CalculateScoreForMatches(List matches)', validate parameter 'matches' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.
CA1806 get_WindowIcon calls GetWindowThreadProcessId but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
CA1806 get_Title calls GetWindowText but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
CA1806 get_ProcessName calls EnumChildWindows but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
CA1806 get_ClassName calls GetClassName but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
CA1806 GetProcessIDFromWindowHandle calls GetWindowThreadProcessId but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
CA1814 FindBestFuzzyMatch uses a multidimensional array of bool[,]. Replace it with a jagged array if possible.
CA2235 Field NormalPosition is a member of type WINDOWPLACEMENT which is serializable but is of type Microsoft.Plugin.WindowWalker.Components.InteropAndHelpers.RECT which is not serializable
CA2235 Field MinPosition is a member of type WINDOWPLACEMENT which is serializable but is of type Microsoft.Plugin.WindowWalker.Components.InteropAndHelpers.POINT which is not serializable
CA2235 Field MaxPosition is a member of type WINDOWPLACEMENT which is serializable but is of type Microsoft.Plugin.WindowWalker.Components.InteropAndHelpers.POINT which is not serializable
CA1806 DeactivateLivePreview calls DwmpActivateLivePreview but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
CA2007 Consider calling ConfigureAwait on the awaited task
CA2007 Consider calling ConfigureAwait on the awaited task
CA2007 Consider calling ConfigureAwait on the awaited task
CA1806 ActivateLivePreview calls DwmpActivateLivePreview but does not use the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.

Validation Steps Performed

How does someone test & validate?

…class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it static (Shared in Visual Basic).
…se the HRESULT or error code that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.
…AndHelpers.RECT should implement IEquatable<T> because it overrides Equals
https://docs.microsoft.com/en-us/windows/win32/winmsg/extended-window-styles.

CA1069 The enum member 'WS_EX_LTRREADING' has the same constant value '0' as member 'WS_EX_LEFT'
CA1069 The enum member 'WS_EX_RIGHTSCROLLBAR' has the same constant value '0' as member 'WS_EX_LEFT'
Code Description
CA1069 The enum member 'SWP_NOREPOSITION' has the same constant value '512' as member 'SWP_NOOWNERZORDER'
CA1069 The enum member 'SWP_FRAMECHANGED' has the same constant value '32' as member 'SWP_DRAWFRAME'
https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-showwindow

CA1069 The enum member 'ShowMaximized' has the same constant value '3' as member 'Maximize'
CA2235 Field MinPosition is a member of type WINDOWPLACEMENT which is serializable but is of type Microsoft.Plugin.WindowWalker.Components.InteropAndHelpers.POINT which is not serializable
CA2235 Field MaxPosition is a member of type WINDOWPLACEMENT which is serializable but is of type Microsoft.Plugin.WindowWalker.Components.InteropAndHelpers.POINT which is not serializable
CA2235 Field NormalPosition is a member of type WINDOWPLACEMENT which is serializable but is of type Microsoft.Plugin.WindowWalker.Components.InteropAndHelpers.RECT which is not serializable
CA1822 Member 'OnOpenWindowsUpdate' does not access instance data and can be marked as static
Code Description
CA1801 Parameter value of method add_OnOpenWindowsUpdate is never used. Remove the parameter or use it in the method body.
CA1801 Parameter value of method remove_OnOpenWindowsUpdate is never used. Remove the parameter or use it in the method body.
…BestFuzzyMatch(string text, string searchText)', validate parameter 'searchText' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.
…on the current user's locale settings.

CA1304 The behavior of 'string.ToLower()' could vary based on the current user's locale settings. Replace this call in 'FuzzyMatching.FindBestFuzzyMatch(string, string)' with a call to 'string.ToLower(CultureInfo)'.

Code Description
CA1304 The behavior of 'string.ToLower()' could vary based on the current user's locale settings. Replace this call in 'FuzzyMatching.FindBestFuzzyMatch(string, string)' with a call to 'string.ToLower(CultureInfo)'.
…onal however this might be something to consider if needing to optimize the window walker search.
…ng.GetAllMatchIndexes(bool[,] matches)', validate parameter 'matches' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.
…ateScoreForMatches(List<int> matches)', validate parameter 'matches' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.
…e that the method returns. This could lead to unexpected behavior in error conditions or low-resource situations. Use the result in a conditional statement, assign the result to a variable, or pass it as an argument to another method.

Using discard for methods that return void, and checking the hresult before returning parameters.
…or 'string.IsNullOrEmpty' method instead of an Equality check
CA1062 In externally visible method 'List<Result> Main.Query(Query query)', validate parameter 'query' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.
…on the current user's locale settings.

CA1304 The behavior of 'string.ToLower()' could vary based on the current user's locale settings. Replace this call in 'SearchController.SearchText.set' with a call to 'string.ToLower(CultureInfo)'.
CA1304 The behavior of 'string.ToLower()' could vary based on the current user's locale settings. Replace this call in 'Window.ProcessName.get' with a call to 'string.ToLower(CultureInfo)'.
CA1304 The behavior of 'string.ToLower()' could vary based on the current user's locale settings. Replace this call in 'Window.SwitchToWindow()' with a call to 'string.ToLower(CultureInfo)'.
CA1304 The behavior of 'string.ToUpper()' could vary based on the current user's locale settings. Replace this call in 'Window.ToString()' with a call to 'string.ToUpper(CultureInfo)'.
CA1307 The behavior of 'string.Equals(string?)' could vary based on the current user's locale settings. Replace this call in 'Microsoft.Plugin.WindowWalker.Components.Window.SwitchToWindow()' with a call to 'string.Equals(string?, System.StringComparison)'.
…s never used

1) Remove SearchController::OpenWindowsUpdateHandler(object sender, SearchResultUpdateEventArgs e) as it wasn't being called and was redundant with Update Search Text.
2) In Main.cs calling UpdateOpenWindowsList before UpdateSearchText so that the latest enumerated windows will be called.
3) Removing unused OnOpenWindowsUpdateEventHandler and related code.
…al return value is number of characters written.

NativeMethods.GetWindowText(hwnd, titleBuffer, sizeOfTitle);
…al return value is number of characters written.

NativeMethods.GetClassName(Hwnd, windowClassName, windowClassName.MaxCapacity);
@ryanbodrug-microsoft ryanbodrug-microsoft marked this pull request as ready for review September 4, 2020 16:09
Copy link
Contributor

@alekhyareddy28 alekhyareddy28 left a comment

Choose a reason for hiding this comment

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

LGTM

@ryanbodrug-microsoft ryanbodrug-microsoft merged commit c4cb3df into microsoft:master Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants