My codespell dictionary has a lot of capitalized words. For example:
MSDOS->MS-DOS
Since checkpatch uses case-insensitive matching, I get an undefined
variable warning and then empty suggestions for things like this:
Use of uninitialized value $typo_fix in concatenation (.) or string at ./scripts/checkpatch.pl line 2958.
WARNING: 'msdos' may be misspelled - perhaps ''?
+ struct msdos_dir_entry *de;
This fixes the matcher to avoid the warning, but it's still a rather
silly suggestion:
WARNING: 'msdos' may be misspelled - perhaps 'MS-DOS'?
+ struct msdos_dir_entry *de;
So I'm not really sure what to do with this ... filter out bad
suggestions instead?
Cc: Andy Whitcroft <[email protected]>
Cc: Joe Perches <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e9f8146600d0..1aaf3317b6ad 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -727,7 +727,7 @@ if ($codespell) {
my ($suspect, $fix) = split(/->/, $line);
- $spelling_fix{$suspect} = $fix;
+ $spelling_fix{lc($suspect)} = $fix;
}
close($spelling);
} else {
--
2.25.1
--
Kees Cook
On Wed, 2020-06-03 at 16:19 -0700, Kees Cook wrote:
> My codespell dictionary has a lot of capitalized words. For example:
>
> MSDOS->MS-DOS
>
> Since checkpatch uses case-insensitive matching, I get an undefined
> variable warning and then empty suggestions for things like this:
>
> Use of uninitialized value $typo_fix in concatenation (.) or string at ./scripts/checkpatch.pl line 2958.
>
> WARNING: 'msdos' may be misspelled - perhaps ''?
> + struct msdos_dir_entry *de;
>
> This fixes the matcher to avoid the warning, but it's still a rather
> silly suggestion:
>
> WARNING: 'msdos' may be misspelled - perhaps 'MS-DOS'?
> + struct msdos_dir_entry *de;
>
> So I'm not really sure what to do with this ... filter out bad
> suggestions instead?
Hey Kees.
Maybe this?
btw: My codespell dictionary file moved to
/usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt
and I had to use --codespell --codespellfile=(above) so
maybe there should be multiple lookups for this file
like the array below.
Are there other standard codespell dictionary locations?
---
scripts/checkpatch.pl | 31 ++++++++++++++++++++++++++-----
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5f00df2c3f59..52aa0dd53d80 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -59,7 +59,7 @@ my $minimum_perl_version = 5.10.0;
my $min_conf_desc_length = 4;
my $spelling_file = "$D/spelling.txt";
my $codespell = 0;
-my $codespellfile = "/usr/share/codespell/dictionary.txt";
+my $codespellfile;
my $conststructsfile = "$D/const_structs.checkpatch";
my $typedefsfile = "";
my $color = "auto";
@@ -716,7 +716,20 @@ if (open(my $spelling, '<', $spelling_file)) {
}
if ($codespell) {
- if (open(my $spelling, '<', $codespellfile)) {
+ if (!defined($codespellfile)) {
+ my @csfiles = ("/usr/share/codespell/dictionary.txt",
+ "/usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt");
+ foreach my $csfile (@csfiles) {
+ if (-f $csfile) {
+ $codespellfile = $csfile;
+ last;
+ }
+ }
+ }
+
+ if (!defined($codespellfile)) {
+ warn "No codespell typos will be found - codespell dictionary not found\n";
+ } elsif (open(my $spelling, '<', $codespellfile)) {
while (<$spelling>) {
my $line = $_;
@@ -2963,13 +2976,21 @@ sub process {
while ($rawline =~ /(?:^|[^a-z@])($misspellings)(?:\b|$|[^a-z@])/gi) {
my $typo = $1;
my $typo_fix = $spelling_fix{lc($typo)};
- $typo_fix = ucfirst($typo_fix) if ($typo =~ /^[A-Z]/);
- $typo_fix = uc($typo_fix) if ($typo =~ /^[A-Z]+$/);
+ $typo_fix = $spelling_fix{$typo} if (!defined($typo_fix));
+ $typo_fix = $spelling_fix{uc($typo)} if (!defined($typo_fix));
+ $typo_fix = 'unknown typo fix' if (!defined($typo_fix));
+ if ($typo =~ /^[A-Z]+$/) {
+ $typo_fix = uc($typo_fix);
+ } elsif ($typo =~ /^[A-Z]/) {
+ $typo_fix = ucfirst($typo_fix);
+ }
+
my $msg_level = \&WARN;
$msg_level = \&CHK if ($file);
if (&{$msg_level}("TYPO_SPELLING",
"'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $herecurr) &&
- $fix) {
+ $fix &&
+ $typo_fix ne 'unknown typo fix') {
$fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($typo)($|[^A-Za-z@])/$1$typo_fix$3/;
}
}
On Thu, 4 Jun 2020 at 03:39, Joe Perches <[email protected]> wrote:
>
> On Wed, 2020-06-03 at 16:19 -0700, Kees Cook wrote:
> > My codespell dictionary has a lot of capitalized words. For example:
> >
> > MSDOS->MS-DOS
> >
> > Since checkpatch uses case-insensitive matching, I get an undefined
> > variable warning and then empty suggestions for things like this:
> >
> > Use of uninitialized value $typo_fix in concatenation (.) or string at ./scripts/checkpatch.pl line 2958.
> >
> > WARNING: 'msdos' may be misspelled - perhaps ''?
> > + struct msdos_dir_entry *de;
> >
> > This fixes the matcher to avoid the warning, but it's still a rather
> > silly suggestion:
> >
> > WARNING: 'msdos' may be misspelled - perhaps 'MS-DOS'?
> > + struct msdos_dir_entry *de;
> >
> > So I'm not really sure what to do with this ... filter out bad
> > suggestions instead?
>
> Hey Kees.
>
> Maybe this?
>
> btw: My codespell dictionary file moved to
> /usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt
>
> and I had to use --codespell --codespellfile=(above) so
> maybe there should be multiple lookups for this file
> like the array below.
>
> Are there other standard codespell dictionary locations?
It might be better to support standard and non standard locations. I
think it's better to request from codespell where his dictionary is.
I created ticket for this:
https://github.com/codespell-project/codespell/issues/1540
However this patch is good as a temporary solution.
Regards,
Maxim.
> ---
> scripts/checkpatch.pl | 31 ++++++++++++++++++++++++++-----
> 1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 5f00df2c3f59..52aa0dd53d80 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -59,7 +59,7 @@ my $minimum_perl_version = 5.10.0;
> my $min_conf_desc_length = 4;
> my $spelling_file = "$D/spelling.txt";
> my $codespell = 0;
> -my $codespellfile = "/usr/share/codespell/dictionary.txt";
> +my $codespellfile;
> my $conststructsfile = "$D/const_structs.checkpatch";
> my $typedefsfile = "";
> my $color = "auto";
> @@ -716,7 +716,20 @@ if (open(my $spelling, '<', $spelling_file)) {
> }
>
> if ($codespell) {
> - if (open(my $spelling, '<', $codespellfile)) {
> + if (!defined($codespellfile)) {
> + my @csfiles = ("/usr/share/codespell/dictionary.txt",
> + "/usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt");
> + foreach my $csfile (@csfiles) {
> + if (-f $csfile) {
> + $codespellfile = $csfile;
> + last;
> + }
> + }
> + }
> +
> + if (!defined($codespellfile)) {
> + warn "No codespell typos will be found - codespell dictionary not found\n";
> + } elsif (open(my $spelling, '<', $codespellfile)) {
> while (<$spelling>) {
> my $line = $_;
>
> @@ -2963,13 +2976,21 @@ sub process {
> while ($rawline =~ /(?:^|[^a-z@])($misspellings)(?:\b|$|[^a-z@])/gi) {
> my $typo = $1;
> my $typo_fix = $spelling_fix{lc($typo)};
> - $typo_fix = ucfirst($typo_fix) if ($typo =~ /^[A-Z]/);
> - $typo_fix = uc($typo_fix) if ($typo =~ /^[A-Z]+$/);
> + $typo_fix = $spelling_fix{$typo} if (!defined($typo_fix));
> + $typo_fix = $spelling_fix{uc($typo)} if (!defined($typo_fix));
> + $typo_fix = 'unknown typo fix' if (!defined($typo_fix));
> + if ($typo =~ /^[A-Z]+$/) {
> + $typo_fix = uc($typo_fix);
> + } elsif ($typo =~ /^[A-Z]/) {
> + $typo_fix = ucfirst($typo_fix);
> + }
> +
> my $msg_level = \&WARN;
> $msg_level = \&CHK if ($file);
> if (&{$msg_level}("TYPO_SPELLING",
> "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $herecurr) &&
> - $fix) {
> + $fix &&
> + $typo_fix ne 'unknown typo fix') {
> $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($typo)($|[^A-Za-z@])/$1$typo_fix$3/;
> }
> }
>
On Thu, 2020-06-04 at 09:55 +0300, Maxim Uvarov wrote:
> On Thu, 4 Jun 2020 at 03:39, Joe Perches <[email protected]> wrote:
Hi Maxim.
> > btw: My codespell dictionary file moved to
> > /usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt
> >
> > and I had to use --codespell --codespellfile=(above) so
> > maybe there should be multiple lookups for this file
> > like the array below.
> >
> > Are there other standard codespell dictionary locations?
>
> It might be better to support standard and non standard locations.
It already does with the --codespellfile=<location> opti.
> I think it's better to request from codespell where his dictionary is.
Maybe a good idea, but looking at the codespell git, for
versions 1.17 on there are several standard dictionaries.
https://github.com/codespell-project/codespell/tree/v1.17.1/codespell_lib/data
> I created ticket for this:
> https://github.com/codespell-project/codespell/issues/1540
Even if codespell is updated, the script would have to deal
with older versions that don't support requesting that option.
cheers, Joe
On Thu, 4 Jun 2020 at 10:29, Joe Perches <[email protected]> wrote:
>
> On Thu, 2020-06-04 at 09:55 +0300, Maxim Uvarov wrote:
> > On Thu, 4 Jun 2020 at 03:39, Joe Perches <[email protected]> wrote:
>
> Hi Maxim.
>
> > > btw: My codespell dictionary file moved to
> > > /usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt
> > >
> > > and I had to use --codespell --codespellfile=(above) so
> > > maybe there should be multiple lookups for this file
> > > like the array below.
> > >
> > > Are there other standard codespell dictionary locations?
> >
> > It might be better to support standard and non standard locations.
>
> It already does with the --codespellfile=<location> opti.
>
I mean if codespess is in your PATH then it will be good to query
dictinary.txt. Even if you install codespell to your $HOME it still is
possible to use it without manually specifying where is the file.
> > I think it's better to request from codespell where his dictionary is.
>
> Maybe a good idea, but looking at the codespell git, for
> versions 1.17 on there are several standard dictionaries.
> https://github.com/codespell-project/codespell/tree/v1.17.1/codespell_lib/data
>
> > I created ticket for this:
> > https://github.com/codespell-project/codespell/issues/1540
>
> Even if codespell is updated, the script would have to deal
> with older versions that don't support requesting that option.
>
Yes, we have to support old versions.
> cheers, Joe
>
>
On Wed, Jun 03, 2020 at 05:39:47PM -0700, Joe Perches wrote:
> On Wed, 2020-06-03 at 16:19 -0700, Kees Cook wrote:
> > My codespell dictionary has a lot of capitalized words. For example:
> >
> > MSDOS->MS-DOS
> >
> > Since checkpatch uses case-insensitive matching, I get an undefined
> > variable warning and then empty suggestions for things like this:
> >
> > Use of uninitialized value $typo_fix in concatenation (.) or string at ./scripts/checkpatch.pl line 2958.
> >
> > WARNING: 'msdos' may be misspelled - perhaps ''?
> > + struct msdos_dir_entry *de;
> >
> > This fixes the matcher to avoid the warning, but it's still a rather
> > silly suggestion:
> >
> > WARNING: 'msdos' may be misspelled - perhaps 'MS-DOS'?
> > + struct msdos_dir_entry *de;
> >
> > So I'm not really sure what to do with this ... filter out bad
> > suggestions instead?
>
> Hey Kees.
>
> Maybe this?
>
> btw: My codespell dictionary file moved to
> /usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt
Yeah, mine too. I think I may have added a symlink to my filesystem to
work around this.
> and I had to use --codespell --codespellfile=(above) so
> maybe there should be multiple lookups for this file
> like the array below.
That seems like a good idea.
>
> Are there other standard codespell dictionary locations?
> ---
> scripts/checkpatch.pl | 31 ++++++++++++++++++++++++++-----
> 1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 5f00df2c3f59..52aa0dd53d80 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -59,7 +59,7 @@ my $minimum_perl_version = 5.10.0;
> my $min_conf_desc_length = 4;
> my $spelling_file = "$D/spelling.txt";
> my $codespell = 0;
> -my $codespellfile = "/usr/share/codespell/dictionary.txt";
> +my $codespellfile;
> my $conststructsfile = "$D/const_structs.checkpatch";
> my $typedefsfile = "";
> my $color = "auto";
> @@ -716,7 +716,20 @@ if (open(my $spelling, '<', $spelling_file)) {
> }
>
> if ($codespell) {
> - if (open(my $spelling, '<', $codespellfile)) {
> + if (!defined($codespellfile)) {
> + my @csfiles = ("/usr/share/codespell/dictionary.txt",
> + "/usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt");
> + foreach my $csfile (@csfiles) {
> + if (-f $csfile) {
> + $codespellfile = $csfile;
> + last;
> + }
> + }
> + }
> +
> + if (!defined($codespellfile)) {
> + warn "No codespell typos will be found - codespell dictionary not found\n";
> + } elsif (open(my $spelling, '<', $codespellfile)) {
> while (<$spelling>) {
> my $line = $_;
>
> @@ -2963,13 +2976,21 @@ sub process {
> while ($rawline =~ /(?:^|[^a-z@])($misspellings)(?:\b|$|[^a-z@])/gi) {
> my $typo = $1;
> my $typo_fix = $spelling_fix{lc($typo)};
> - $typo_fix = ucfirst($typo_fix) if ($typo =~ /^[A-Z]/);
> - $typo_fix = uc($typo_fix) if ($typo =~ /^[A-Z]+$/);
> + $typo_fix = $spelling_fix{$typo} if (!defined($typo_fix));
> + $typo_fix = $spelling_fix{uc($typo)} if (!defined($typo_fix));
This won't catch stuff like:
Cambrige->Cambridge
because neither "cambrige" nor "CAMBRIGE" is in %spelling_fix. And the
original text is lost due to the //i. :( I'm really not sure what to do
with these things in codespell. Lower case everything? Throw away
anything not all lower case?
--
Kees Cook
On Thu, 2020-06-04 at 15:08 -0700, Kees Cook wrote:
> On Wed, Jun 03, 2020 at 05:39:47PM -0700, Joe Perches wrote:
> > On Wed, 2020-06-03 at 16:19 -0700, Kees Cook wrote:
> > > My codespell dictionary has a lot of capitalized words. For example:
> > >
> > > MSDOS->MS-DOS
> > >
> > > Since checkpatch uses case-insensitive matching, I get an undefined
> > > variable warning and then empty suggestions for things like this:
> > >
> > > Use of uninitialized value $typo_fix in concatenation (.) or string at ./scripts/checkpatch.pl line 2958.
> > >
> > > WARNING: 'msdos' may be misspelled - perhaps ''?
> > > + struct msdos_dir_entry *de;
> > >
> > > This fixes the matcher to avoid the warning, but it's still a rather
> > > silly suggestion:
> > >
> > > WARNING: 'msdos' may be misspelled - perhaps 'MS-DOS'?
> > > + struct msdos_dir_entry *de;
> > >
> > > So I'm not really sure what to do with this ... filter out bad
> > > suggestions instead?
> >
> > Hey Kees.
> >
> > Maybe this?
> >
> > btw: My codespell dictionary file moved to
> > /usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt
>
> Yeah, mine too. I think I may have added a symlink to my filesystem to
> work around this.
>
> > and I had to use --codespell --codespellfile=(above) so
> > maybe there should be multiple lookups for this file
> > like the array below.
>
> That seems like a good idea.
>
> > Are there other standard codespell dictionary locations?
> > ---
> > scripts/checkpatch.pl | 31 ++++++++++++++++++++++++++-----
> > 1 file changed, 26 insertions(+), 5 deletions(-)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 5f00df2c3f59..52aa0dd53d80 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -59,7 +59,7 @@ my $minimum_perl_version = 5.10.0;
> > my $min_conf_desc_length = 4;
> > my $spelling_file = "$D/spelling.txt";
> > my $codespell = 0;
> > -my $codespellfile = "/usr/share/codespell/dictionary.txt";
> > +my $codespellfile;
> > my $conststructsfile = "$D/const_structs.checkpatch";
> > my $typedefsfile = "";
> > my $color = "auto";
> > @@ -716,7 +716,20 @@ if (open(my $spelling, '<', $spelling_file)) {
> > }
> >
> > if ($codespell) {
> > - if (open(my $spelling, '<', $codespellfile)) {
> > + if (!defined($codespellfile)) {
> > + my @csfiles = ("/usr/share/codespell/dictionary.txt",
> > + "/usr/lib/python3/dist-packages/codespell_lib/data/dictionary.txt");
> > + foreach my $csfile (@csfiles) {
> > + if (-f $csfile) {
> > + $codespellfile = $csfile;
> > + last;
> > + }
> > + }
> > + }
> > +
> > + if (!defined($codespellfile)) {
> > + warn "No codespell typos will be found - codespell dictionary not found\n";
> > + } elsif (open(my $spelling, '<', $codespellfile)) {
> > while (<$spelling>) {
> > my $line = $_;
> >
> > @@ -2963,13 +2976,21 @@ sub process {
> > while ($rawline =~ /(?:^|[^a-z@])($misspellings)(?:\b|$|[^a-z@])/gi) {
> > my $typo = $1;
> > my $typo_fix = $spelling_fix{lc($typo)};
> > - $typo_fix = ucfirst($typo_fix) if ($typo =~ /^[A-Z]/);
> > - $typo_fix = uc($typo_fix) if ($typo =~ /^[A-Z]+$/);
> > + $typo_fix = $spelling_fix{$typo} if (!defined($typo_fix));
> > + $typo_fix = $spelling_fix{uc($typo)} if (!defined($typo_fix));
>
> This won't catch stuff like:
>
> Cambrige->Cambridge
Huh? Did you test this?
$ git diff scripts/spelling.txt
diff --git a/scripts/spelling.txt b/scripts/spelling.txt
index d9cd24cf0d40..43c3332935fe 100644
--- a/scripts/spelling.txt
+++ b/scripts/spelling.txt
@@ -251,6 +251,7 @@ callibration||calibration
callled||called
calucate||calculate
calulate||calculate
+Cambridg||Cambridge
cancelation||cancellation
cancle||cancel
capabilites||capabilities
$ cat test_spell.c
// SPDX-License-Identifier: GPL-2.0-only
/* This is a test.
* Cambridg
*/
$ ./scripts/checkpatch.pl --strict -f test_spell.c
CHECK: 'Cambridg' may be misspelled - perhaps 'Cambridge'?
#4: FILE: test_spell.c:4:
+ * Cambridg
total: 0 errors, 0 warnings, 1 checks, 5 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
test_spell.c has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
> because neither "cambrige" nor "CAMBRIGE" is in %spelling_fix. And the
> original text is lost due to the //i. :( I'm really not sure what to do
> with these things in codespell. Lower case everything? Throw away
> anything not all lower case?
On Thu, Jun 04, 2020 at 06:02:05PM -0700, Joe Perches wrote:
> Huh? Did you test this?
I didn't, no. I was going off my earlier discoveries about how the
"msdos" thing got parsed weird. My apologies!
> $ ./scripts/checkpatch.pl --strict -f test_spell.c
> CHECK: 'Cambridg' may be misspelled - perhaps 'Cambridge'?
> #4: FILE: test_spell.c:4:
> + * Cambridg
Thanks for sorting this (and me) out! :)
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook