From 7c480c0d4fc61c322fcee0b80e241e064deedb0b Mon Sep 17 00:00:00 2001 From: k1LoW Date: Wed, 20 Sep 2023 20:29:26 +0900 Subject: [PATCH 1/3] Add errorstrings analyzer ref: https://github.com/golang/go/wiki/CodeReviewComments#error-strings --- _REFERENCE_STYLE_CREDITS | 35 ++++- .../errorstrings/errorstrings.go | 127 ++++++++++++++++++ .../errorstrings/errorstrings_test.go | 14 ++ .../errorstrings/testdata/src/a/a.go | 17 +++ .../errorstrings/testdata/src/a/go.mod | 4 + config/config.go | 31 +++-- main.go | 2 + 7 files changed, 216 insertions(+), 14 deletions(-) create mode 100644 analyzer/code_review_comments/errorstrings/errorstrings.go create mode 100644 analyzer/code_review_comments/errorstrings/errorstrings_test.go create mode 100644 analyzer/code_review_comments/errorstrings/testdata/src/a/a.go create mode 100644 analyzer/code_review_comments/errorstrings/testdata/src/a/go.mod diff --git a/_REFERENCE_STYLE_CREDITS b/_REFERENCE_STYLE_CREDITS index f2948e2..18add34 100644 --- a/_REFERENCE_STYLE_CREDITS +++ b/_REFERENCE_STYLE_CREDITS @@ -8,6 +8,39 @@ Effective Go by The Go Authors is licensed under CC BY 4.0 (https://creativecomm Google Style Guides https://google.github.io/styleguide/ ---------------------------------------------------------------- - Google Style Guides by Google is licensed under CC BY 3.0 (https://creativecommons.org/licenses/by/3.0/) + +================================================================ + +Go wiki (Go Code Review Comments) +https://github.com/golang/go/wiki (https://github.com/golang/go/wiki/CodeReviewComments) +---------------------------------------------------------------- +Copyright (c) 2009 The Go Authors. All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + * Neither the name of Google Inc. nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + ================================================================ diff --git a/analyzer/code_review_comments/errorstrings/errorstrings.go b/analyzer/code_review_comments/errorstrings/errorstrings.go new file mode 100644 index 0000000..a9729ec --- /dev/null +++ b/analyzer/code_review_comments/errorstrings/errorstrings.go @@ -0,0 +1,127 @@ +package errorstrings + +import ( + "fmt" + "go/ast" + "strings" + "unicode" + + "github.com/gostaticanalysis/comment/passes/commentmap" + "github.com/k1LoW/gostyle/config" + "github.com/k1LoW/gostyle/reporter" + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +const ( + name = "errorstrings" + doc = "Analyzer based on https://github.com/golang/go/wiki/CodeReviewComments#error-strings" + msg = "Error strings should not be capitalized (unless beginning with proper nouns or acronyms) or end with punctuation, since they are usually printed following other context. (ref: https://github.com/golang/go/wiki/CodeReviewComments#error-strings)" +) + +var ( + disable bool + includeGenerated bool +) + +// Analyzer based on https://github.com/golang/go/wiki/CodeReviewComments#error-strings +var Analyzer = &analysis.Analyzer{ + Name: name, + Doc: doc, + Run: run, + Requires: []*analysis.Analyzer{ + inspect.Analyzer, + commentmap.Analyzer, + }, +} + +// AnalyzerWithConfig based on https://github.com/golang/go/wiki/CodeReviewComments#error-strings +var AnalyzerWithConfig = &analysis.Analyzer{ + Name: name, + Doc: doc, + Run: run, + Requires: []*analysis.Analyzer{ + config.Loader, + inspect.Analyzer, + commentmap.Analyzer, + }, +} + +func run(pass *analysis.Pass) (any, error) { + c, err := config.Load(pass) + if err != nil { + return nil, err + } + if c != nil { + disable = c.IsDisabled(name) + includeGenerated = c.AnalyzersSettings.Errorstrings.IncludeGenerated + } + if disable { + return nil, nil + } + i, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + if !ok { + return nil, fmt.Errorf("unexpected result type from inspect: %T", pass.ResultOf[inspect.Analyzer]) + } + + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), + } + + var opts []reporter.Option + if includeGenerated { + opts = append(opts, reporter.IncludeGenerated()) + } + r, err := reporter.New(name, pass, opts...) + if err != nil { + return nil, err + } + i.Preorder(nodeFilter, func(n ast.Node) { + switch e := n.(type) { + case *ast.CallExpr: + if len(e.Args) == 0 { + return + } + fn, ok := e.Fun.(*ast.SelectorExpr) + if !ok { + return + } + if fn.Sel.Name == "Errorf" { + bl, ok := e.Args[0].(*ast.BasicLit) + if !ok { + return + } + if isNG(bl.Value) { + r.Append(e.Pos(), fmt.Sprintf("%s: %s", msg, bl.Value)) + } + return + } + id, ok := fn.X.(*ast.Ident) + if !ok { + return + } + if id.Name == "errors" && fn.Sel.Name == "New" { + bl, ok := e.Args[0].(*ast.BasicLit) + if !ok { + return + } + if isNG(bl.Value) { + r.Append(e.Pos(), fmt.Sprintf("%s: %s", msg, bl.Value)) + } + } + } + }) + r.Report() + return nil, nil +} + +func isNG(in string) bool { + format := strings.Trim(in, "\"'`") + return unicode.IsUpper(rune(format[0])) || strings.HasSuffix(format, ".") +} + +func init() { + Analyzer.Flags.BoolVar(&disable, "disable", false, "disable "+name+" analyzer") + Analyzer.Flags.BoolVar(&includeGenerated, "include-generated", false, "include generated codes") +} diff --git a/analyzer/code_review_comments/errorstrings/errorstrings_test.go b/analyzer/code_review_comments/errorstrings/errorstrings_test.go new file mode 100644 index 0000000..a5bff02 --- /dev/null +++ b/analyzer/code_review_comments/errorstrings/errorstrings_test.go @@ -0,0 +1,14 @@ +package errorstrings + +import ( + "testing" + + "github.com/gostaticanalysis/testutil" + "golang.org/x/tools/go/analysis/analysistest" +) + +// TestAnalyzer is a test for Analyzer. +func TestAnalyzer(t *testing.T) { + td := testutil.WithModules(t, analysistest.TestData(), nil) + analysistest.Run(t, td, Analyzer, "a") +} diff --git a/analyzer/code_review_comments/errorstrings/testdata/src/a/a.go b/analyzer/code_review_comments/errorstrings/testdata/src/a/a.go new file mode 100644 index 0000000..d0f2724 --- /dev/null +++ b/analyzer/code_review_comments/errorstrings/testdata/src/a/a.go @@ -0,0 +1,17 @@ +package a + +import ( + "errors" + "fmt" +) + +func f() { + e := fmt.Errorf("This is %s", "world") // want "gostyle.errorstrings" + print(e.Error()) + e2 := fmt.Errorf("this is %s.", "world") // want "gostyle.errorstrings" + print(e2.Error()) + var e3 = errors.New("This is world") // want "gostyle.errorstrings" + print(e3.Error()) + var e4 = errors.New("this is world.") // want "gostyle.errorstrings" + print(e4.Error()) +} diff --git a/analyzer/code_review_comments/errorstrings/testdata/src/a/go.mod b/analyzer/code_review_comments/errorstrings/testdata/src/a/go.mod new file mode 100644 index 0000000..f8a6f85 --- /dev/null +++ b/analyzer/code_review_comments/errorstrings/testdata/src/a/go.mod @@ -0,0 +1,4 @@ +module a + +go 1.21 + diff --git a/config/config.go b/config/config.go index bd1e446..eddb4f4 100644 --- a/config/config.go +++ b/config/config.go @@ -37,19 +37,24 @@ type Analyzers struct { } type AnalyzersSettings struct { - Getters Getters `yaml:"getters"` - Ifacenames Ifacenames `yaml:"ifacenames"` - Mixedcaps Mixedcaps `yaml:"mixedcaps"` - Nilslices Nilslices `yaml:"nilslices"` - Pkgnames Pkgnames `yaml:"pkgnames"` - Recvnames Recvnames `yaml:"recvnames"` - Recvtype Recvtype `yaml:"recvtype"` - Repetition Repetition `yaml:"repetition"` - Typealiases Typealiases `yaml:"typealiases"` - Underscores Underscores `yaml:"underscores"` - Useany Useany `yaml:"useany"` - Useq Useq `yaml:"useq"` - Varnames Varnames `yaml:"varnames"` + Errorstrings Errorstrings `yaml:"errorstrings"` + Getters Getters `yaml:"getters"` + Ifacenames Ifacenames `yaml:"ifacenames"` + Mixedcaps Mixedcaps `yaml:"mixedcaps"` + Nilslices Nilslices `yaml:"nilslices"` + Pkgnames Pkgnames `yaml:"pkgnames"` + Recvnames Recvnames `yaml:"recvnames"` + Recvtype Recvtype `yaml:"recvtype"` + Repetition Repetition `yaml:"repetition"` + Typealiases Typealiases `yaml:"typealiases"` + Underscores Underscores `yaml:"underscores"` + Useany Useany `yaml:"useany"` + Useq Useq `yaml:"useq"` + Varnames Varnames `yaml:"varnames"` +} + +type Errorstrings struct { + IncludeGenerated bool `yaml:"include-generated"` } type Getters struct { diff --git a/main.go b/main.go index 361307e..11e5774 100644 --- a/main.go +++ b/main.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" + "github.com/k1LoW/gostyle/analyzer/code_review_comments/errorstrings" "github.com/k1LoW/gostyle/analyzer/decisions/getters" "github.com/k1LoW/gostyle/analyzer/decisions/nilslices" "github.com/k1LoW/gostyle/analyzer/decisions/pkgnames" @@ -40,6 +41,7 @@ func main() { unitchecker.Main( config.Loader, + errorstrings.AnalyzerWithConfig, getters.AnalyzerWithConfig, ifacenames.AnalyzerWithConfig, pkgnames.AnalyzerWithConfig, From c9feda5c5d47cd94f4139dbc3a07958b2b56e564 Mon Sep 17 00:00:00 2001 From: k1LoW Date: Wed, 20 Sep 2023 20:31:51 +0900 Subject: [PATCH 2/3] Add README --- README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/README.md b/README.md index 8c085aa..34ca387 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,10 @@ jobs: > ["Google Style Guides"](https://google.github.io/styleguide/) by Google is licensed under [CC BY 3.0](https://creativecommons.org/licenses/by/3.0/) +### [Go Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments) in Go wiki + +- [errorstrings](analyzer/comments/errorstrings) ... based on https://github.com/golang/go/wiki/CodeReviewComments#error-strings + ## Disabling and Ignoring ### Disable analyzer @@ -108,6 +112,14 @@ analyzers: ### `analyzers-settings:` +#### errorstrings + +```yaml +analyzers-settings: + errorstrings: + include-generated: false # include generated codes (default: false) +``` + #### getters ```yaml From e995f73d95e5ba7b4d765741e029d43b96da060a Mon Sep 17 00:00:00 2001 From: k1LoW Date: Wed, 20 Sep 2023 20:51:34 +0900 Subject: [PATCH 3/3] Fix lint warn --- analyzer/code_review_comments/errorstrings/errorstrings.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/analyzer/code_review_comments/errorstrings/errorstrings.go b/analyzer/code_review_comments/errorstrings/errorstrings.go index a9729ec..1bcaf60 100644 --- a/analyzer/code_review_comments/errorstrings/errorstrings.go +++ b/analyzer/code_review_comments/errorstrings/errorstrings.go @@ -117,8 +117,8 @@ func run(pass *analysis.Pass) (any, error) { } func isNG(in string) bool { - format := strings.Trim(in, "\"'`") - return unicode.IsUpper(rune(format[0])) || strings.HasSuffix(format, ".") + f := strings.Trim(in, "\"'`") + return unicode.IsUpper(rune(f[0])) || strings.HasSuffix(f, ".") } func init() {