[ColorPicker] Enable analyzer and fix warnings

[CleanCodeDeveloper]

Summary of the Pull Request

What is this about:

What is included in the PR:

How does someone test / validate:

Quality Checklist

• Linked issue: Enable analyzers for each project #16511
• Communication: I've discussed this with core contributors in the issue.
• Tests: Added/updated and all pass
• Installer: Added/updated and all pass
• Localization: All end user facing strings can be localized
• Docs: Added/ updated
• Binaries: Any new files are added to WXS / YML
  • No new binaries
  • YML for signing for new binaries
  • WXS for installer for new binaries

Contributor License Agreement (CLA)

A CLA must be signed. If not, go over here and sign the CLA.

[github-actions]

@check-spelling-bot Report

🔴 Please review

See the files view or the action log for details.

Unrecognized words (1)

postives

Previously acknowledged words that are now absent

abap abcd abcdefgh appcontainer atop autogenerates AZCLI azurecr binskim bios Bools btm buildcommand buildtools cameligo CCB cdpxwin cdxml CEBB CFFDAFADA ChaseKnowlden cjs CleanCodeDeveloper cljs CPPARM CPPx crutkas csx CTLCOLORSTATIC defaultcommand Deuchert dupenv DUSTIN efgh errc errorlevel estdir etcore Firefox FIXME fsscript Gamebar graphql Grayscale gsuberland hmon iccex ICONINFORMATION IMonitor INITCOMMONCONTROLSEX INSTALLLOGATTRIBUTES INSTALLLOGMODE INSTALLUILEVEL Javascript julia Knowlden kotlin ktm kts LASTEXITCODE LEQ ligo linkid litcoffee MAINICON MAKELPARAM Minimizeallwindows mkdir moderncop modulekey msiexec MSIINSTALLER MSIL namings NATIVEFNTCTL neq netlify nocache php phps plx policheck popd powerquery psc psm Pui pushd pyc pyd pyi pyo pyz Qin rda rdata rdeveen rds reportbug rexit robocopy scm SETRANGE SETSTEP SHAREIMAGELISTS sizeg smallbasic spamming spdth sregex STEPIT stx svh SWITCHEND symlink symlinks systemverilog taskkill tbc tcl timezone tlbimp trackpad typeparamref UITo umd uninit UWPUI vbhtml verilog vse vsix WDK wdksetup wdkvsix We'd webclient webpack wifstream WINMSAPP WTL

To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the git@github.com:CleanCodeDeveloper/PowerToys.git repository
on the fix-analyzer-warnings-in-colorpicker branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
-H "Content-Type: application/json" \
"https://api.github.com/repos/microsoft/PowerToys/issues/comments/1049224950" > "$comment_json"
comment_body=$(mktemp)
jq -r ".body // empty" "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")

patch_add=$(perl -e '$/=undef; $_=<>; if (m{Unrecognized words[^<]*</summary>\n*```\n*([^<]*)```\n*</details>$}m) { print "$1" } elsif (m{Unrecognized words[^<]*\n\n((?:\w.*\n)+)\n}m) { print "$1" };' < "$comment_body")

update_files
rm $comment_body
git add -u

If the flagged items do not appear to be text

If items relate to a ...

• well-formed pattern.

  If you can write a pattern that would match it,
  try adding it to the patterns.txt file.

  Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

  Note that patterns can't match multiline strings.

• binary file.

  Please add a file path to the excludes.txt file matching the containing file.

  File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

  ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

[CleanCodeDeveloper]

I'm not sure what to do with CA1838: Avoid StringBuilder parameters for P/Invokes

from the docs:

Suppress a violation of this rule if you're not concerned about the performance impact of marshaling a StringBuilder.

I decided to suppress it for now.

[crutkas]

for stuff supressed, lets create dedicated issues to unsupress

[CleanCodeDeveloper]

for stuff supressed, lets create dedicated issues to unsupress

Sure can do. An alternative approach would be to have a look at the whole GlobalSuppression.cs file later and decide what we want to unsuppress and create issues then.

[jaimecbernardo]

Just a nit. Other than that, LGTM!

[jaimecbernardo]

The logic is changed. Perhaps this needs to be unrolled into an if else block instead, but we should keep logic here.

[CleanCodeDeveloper]

new Exception() raises the warning CA2201: Do not raise reserved exception types.

My though process was that the UnhandledExceptionEventArgs.ExceptionObject will always be of type exception or null. If it is of type exception the code behaves the same.

In the unexpected case that UnhandledExceptionEventArgs.ExceptionObject is not based on Exception or null I changed the code to pass null instead of new Exception() into the LogError(...) Method.

[CleanCodeDeveloper]

What do you think about:

if (e.ExceptionObject is Exception ex)
{
    Logger.LogError("Unhandled exception", ex);
}
else
{
    Logger.LogError("Unhandled exception");
}

instead of

Logger.LogError("Unhandled exception", e.ExceptionObject as Exception);

[jaimecbernardo]

Sounds good to me, actually. Just verified LogError has that alternative signature. No idea why it wasn't used here in the first place 🤷 .
Thank you 😄

[jaimecbernardo]

LGTM! Thank you for the contribution.

[CleanCodeDeveloper]

for stuff supressed, lets create dedicated issues to unsupress

@crutkas : see #16791