diff --git a/hack/tools/go.mod b/hack/tools/go.mod deleted file mode 100644 index 3c94929c9..000000000 --- a/hack/tools/go.mod +++ /dev/null @@ -1,8 +0,0 @@ -module k8s.io/klog/hack/tools - -go 1.15 - -require ( - golang.org/x/exp v0.0.0-20210220032938-85be41e4509f - golang.org/x/tools v0.1.0 -) diff --git a/hack/tools/go.sum b/hack/tools/go.sum deleted file mode 100644 index e89e59cf9..000000000 --- a/hack/tools/go.sum +++ /dev/null @@ -1,46 +0,0 @@ -dmitri.shuralyov.com/gpu/mtl v0.0.0-20201218220906-28db891af037/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= -github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= -github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= -github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/exp v0.0.0-20190731235908-ec7cb31e5a56/go.mod h1:JhuoJpWY28nO4Vef9tZUw9qufEGTyX1+7lmHxV5q5G4= -golang.org/x/exp v0.0.0-20210220032938-85be41e4509f h1:GrkO5AtFUU9U/1f5ctbIBXtBGeSJbWwIYfIsTcFMaX4= -golang.org/x/exp v0.0.0-20210220032938-85be41e4509f/go.mod h1:I6l2HNBLBZEcrOoCpyKLdY2lHoRZ8lI4x60KMCQDft4= -golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= -golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= -golang.org/x/mobile v0.0.0-20190312151609-d3739f865fa6/go.mod h1:z+o9i4GpDbdi3rU15maQ/Ox0txvL9dWGYEHz965HBQE= -golang.org/x/mobile v0.0.0-20201217150744-e6ae53a27f4f/go.mod h1:skQtrUTUwhdJvXM/2KKJzY8pDgNr9I/FOMqDVRPBUS4= -golang.org/x/mod v0.1.0/go.mod h1:0QHyrYULN0/3qlju5TqG8bIK38QM8yzMo5ekMj3DlcY= -golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= -golang.org/x/mod v0.1.1-0.20191209134235-331c550502dd/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.3.1-0.20200828183125-ce943fd02449 h1:xUIPaMhvROX9dhPvRCenIJtU78+lbEenGbgqB5hfHCQ= -golang.org/x/mod v0.3.1-0.20200828183125-ce943fd02449/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= -golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20191001151750-bb3f8db39f24/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k= -golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -golang.org/x/tools v0.0.0-20190312151545-0bb0c0a6e846/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= -golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= -golang.org/x/tools v0.0.0-20200117012304-6edc0a871e69/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= -golang.org/x/tools v0.0.0-20200207183749-b753a1ba74fa/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= -golang.org/x/tools v0.1.0 h1:po9/4sTYwZU9lPhi1tOrb4hCv3qrhiQ77LZfGa2OjwY= -golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= -golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= -golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/hack/tools/logcheck/README.md b/hack/tools/logcheck/README.md deleted file mode 100644 index 1e96afbcd..000000000 --- a/hack/tools/logcheck/README.md +++ /dev/null @@ -1,82 +0,0 @@ -This directory contains a linter for checking log calls. It was originally -created to detect when unstructured logging calls like `klog.Infof` get added -to files that should only use structured logging calls like `klog.InfoS` -and now also supports other checks. - -# Installation - -`go install k8s.io/klog/hack/tools/logcheck` - -# Usage - -`$logcheck.go ` -`e.g $logcheck ./pkg/kubelet/lifecycle/` - -# Configuration - -Checks can be enabled or disabled globally via command line flags and env -variables. In addition, the global setting for a check can be modified per file -via a configuration file. That file contains lines in this format: - -``` - -``` - -`` is a comma-separated list of the names of checks that get enabled or -disabled when a file name matches the regular expression. A check gets disabled -when its name has `-` as prefix and enabled when there is no prefix or `+` as -prefix. Only checks that are mentioned explicitly are modified. All regular -expressions are checked in order, so later lines can override the previous -ones. - -In this example, checking for klog calls is enabled for all files under -`pkg/scheduler` in the Kubernetes repo except for `scheduler.go` -itself. Parameter checking is disabled everywhere. - -``` -klog,-parameters k8s.io/kubernetes/pkg/scheduler/.* --klog k8s.io/kubernetes/pkg/scheduler/scheduler.go -``` - -The names of all supported checks are the ones used as sub-section titles in -the next section. - -# Checks - -## structured (enabled by default) - -Unstructured klog logging calls are flagged as error. - -## klog (disabled by default) - -None of the klog logging methods may be used. This is even stricter than -`unstructured`. Instead, code should retrieve a logr.Logger from klog and log -through that. - -## parameters (enabled by default) - -This ensures that if certain logging functions are allowed and are used, those -functions are passed correct parameters. - -### all calls - -Format strings are not allowed where plain strings are expected. - -### structured logging calls - -Key/value parameters for logging calls are checked: -- For each key there must be a value. -- Keys must be constant strings. - -This also warns about code that is valid, for example code that collects -key/value pairs in an `[]interface` variable before passing that on to a log -call. Such valid code can use `nolint:logcheck` to disable the warning (when -invoking logcheck through golangci-lint) or the `parameters` check can be -disabled for the file. - -## with-helpers (disabled by default) - -`logr.Logger.WithName`, `logr.Logger.WithValues` and `logr.NewContext` must not -be used. The corresponding helper calls from `k8s.io/klogr` should be used -instead. This is relevant when support contextual logging is disabled at -runtime in klog. diff --git a/hack/tools/logcheck/main.go b/hack/tools/logcheck/main.go deleted file mode 100644 index de5fd7e4b..000000000 --- a/hack/tools/logcheck/main.go +++ /dev/null @@ -1,27 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package main - -import ( - "golang.org/x/tools/go/analysis/singlechecker" - - "k8s.io/klog/hack/tools/logcheck/pkg" -) - -func main() { - singlechecker.Main(pkg.Analyser()) -} diff --git a/hack/tools/logcheck/main_test.go b/hack/tools/logcheck/main_test.go deleted file mode 100644 index e759154b5..000000000 --- a/hack/tools/logcheck/main_test.go +++ /dev/null @@ -1,126 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package main - -import ( - "testing" - - "golang.org/x/tools/go/analysis/analysistest" - - "k8s.io/klog/hack/tools/logcheck/pkg" -) - -func TestAnalyzer(t *testing.T) { - tests := []struct { - name string - enabled map[string]string - override string - testPackage string - }{ - { - name: "Allow unstructured logs", - enabled: map[string]string{ - "structured": "false", - }, - testPackage: "allowUnstructuredLogs", - }, - { - name: "Do not allow unstructured logs", - testPackage: "doNotAllowUnstructuredLogs", - }, - { - name: "Per-file config", - enabled: map[string]string{ - "structured": "false", - }, - override: "testdata/src/mixed/structured_logging", - testPackage: "mixed", - }, - { - name: "Function call parameters", - enabled: map[string]string{ - "structured": "false", - }, - testPackage: "parameters", - }, - { - name: "Only allow contextual calls", - enabled: map[string]string{ - "structured": "true", - "contextual": "true", - }, - testPackage: "onlyAllowContextual", - }, - { - name: "Only allow contextual calls through config", - enabled: map[string]string{ - "structured": "false", - "contextual": "false", - }, - override: "testdata/src/onlyAllowContextual/klog_logging", - testPackage: "onlyAllowContextual", - }, - { - name: "importrename", - testPackage: "importrename", - }, - { - name: "verbose", - testPackage: "verbose", - }, - { - name: "gologr", - enabled: map[string]string{ - "contextual": "true", - }, - testPackage: "gologr", - }, - { - name: "contextual", - enabled: map[string]string{ - "contextual": "true", - }, - testPackage: "contextual", - }, - { - name: "helpers", - enabled: map[string]string{ - "with-helpers": "true", - }, - testPackage: "helpers", - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - analyzer := pkg.Analyser() - set := func(flag, value string) { - if value != "" { - if err := analyzer.Flags.Set(flag, value); err != nil { - t.Fatalf("unexpected error for %s: %v", flag, err) - } - } - } - for key, value := range tc.enabled { - set("check-"+key, value) - } - if tc.override != "" { - set("config", tc.override) - } - analysistest.Run(t, analysistest.TestData(), analyzer, tc.testPackage) - }) - } -} diff --git a/hack/tools/logcheck/pkg/filter.go b/hack/tools/logcheck/pkg/filter.go deleted file mode 100644 index c2ad90925..000000000 --- a/hack/tools/logcheck/pkg/filter.go +++ /dev/null @@ -1,129 +0,0 @@ -/* -Copyright 2022 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package pkg - -import ( - "bufio" - "flag" - "fmt" - "os" - "regexp" - "strings" -) - -// RegexpFilter implements flag.Value by accepting a file name and parsing that -// file. -type RegexpFilter struct { - filename string - validChecks map[string]bool - lines []filter -} - -type filter struct { - enabled map[string]bool - match *regexp.Regexp -} - -var _ flag.Value = &RegexpFilter{} - -func (f *RegexpFilter) String() string { - return f.filename -} - -func (f *RegexpFilter) Set(filename string) error { - file, err := os.Open(filename) - if err != nil { - return err - } - defer file.Close() - - // Reset before parsing. - f.filename = filename - f.lines = nil - - // Read line-by-line. - scanner := bufio.NewScanner(file) - for lineNr := 0; scanner.Scan(); lineNr++ { - text := scanner.Text() - if strings.HasPrefix(text, "#") { - continue - } - text = strings.TrimSpace(text) - if text == "" { - continue - } - - line := filter{ - enabled: map[string]bool{}, - } - parts := strings.SplitN(text, " ", 2) - if len(parts) != 2 { - return fmt.Errorf("%s:%d: not of the format : %s", filename, lineNr, text) - } - for _, c := range strings.Split(parts[0], ",") { - enabled := true - if strings.HasPrefix(c, "+") { - c = c[1:] - } else if strings.HasPrefix(c, "-") { - enabled = false - c = c[1:] - } - if !f.validChecks[c] { - return fmt.Errorf("%s:%d: %q is not a supported check: %s", filename, lineNr, c, text) - } - line.enabled[c] = enabled - } - - re, err := regexp.Compile(parts[1]) - if err != nil { - return fmt.Errorf("%s:%d: %v", filename, lineNr, err) - } - line.match = re - f.lines = append(f.lines, line) - } - - if err := scanner.Err(); err != nil { - return err - } - return nil -} - -// Enabled checks whether a certain check is enabled for a file. -func (f *RegexpFilter) Enabled(check string, enabled bool, filename string) bool { - for _, l := range f.lines { - // Must match entire string. - if matchFullString(filename, l.match) { - if e, ok := l.enabled[check]; ok { - enabled = e - } - } - } - return enabled -} - -func matchFullString(str string, re *regexp.Regexp) bool { - loc := re.FindStringIndex(str) - if loc == nil { - // No match at all. - return false - } - if loc[1]-loc[0] < len(str) { - // Only matches a substring. - return false - } - return true -} diff --git a/hack/tools/logcheck/pkg/filter_test.go b/hack/tools/logcheck/pkg/filter_test.go deleted file mode 100644 index eee829a7b..000000000 --- a/hack/tools/logcheck/pkg/filter_test.go +++ /dev/null @@ -1,161 +0,0 @@ -/* -Copyright 2022 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package pkg - -import ( - "io/ioutil" - "path" - "testing" -) - -func TestMatch(t *testing.T) { - temp := t.TempDir() - filename := path.Join(temp, "expressions") - if err := ioutil.WriteFile(filename, []byte(`# Example file -structured hello -+structured a.c --structured adc -structured x.*y -structured,parameters world -`), 0666); err != nil { - t.Fatalf("writing file: %v", err) - } - - filter := &RegexpFilter{ - validChecks: map[string]bool{ - structuredCheck: true, - parametersCheck: true, - }, - } - if err := filter.Set(filename); err != nil { - t.Fatalf("reading file: %v", err) - } - - for _, tc := range []struct { - filename string - check string - enabled bool - expectEnabled bool - }{ - { - filename: "hello", - check: "structured", - expectEnabled: true, - }, - { - filename: "hello", - check: "parameters", - expectEnabled: false, // not set - }, - { - filename: "hello", - check: "parameters", - enabled: true, - expectEnabled: true, // global default - }, - { - filename: "hello/world", - check: "structured", - expectEnabled: false, // no sub-matches - }, - { - filename: "abc", - check: "structured", - expectEnabled: true, - }, - { - filename: "adc", - check: "structured", - expectEnabled: false, // unset later - }, - { - filename: "x1y", - check: "structured", - expectEnabled: true, - }, - { - filename: "x2y", - check: "structured", - expectEnabled: true, - }, - } { - actualEnabled := filter.Enabled(tc.check, tc.enabled, tc.filename) - if actualEnabled != tc.expectEnabled { - t.Errorf("%+v: got %v", tc, actualEnabled) - } - } -} - -func TestSetNoFile(t *testing.T) { - filter := &RegexpFilter{} - if err := filter.Set("no such file"); err == nil { - t.Errorf("did not get expected error") - } -} - -func TestParsing(t *testing.T) { - temp := t.TempDir() - filename := path.Join(temp, "expressions") - for name, tc := range map[string]struct { - content string - expectError string - }{ - "invalid-regexp": { - content: `structured [`, - expectError: filename + ":0: error parsing regexp: missing closing ]: `[`", - }, - "wildcard": { - content: `structured *`, - expectError: filename + ":0: error parsing regexp: missing argument to repetition operator: `*`", - }, - "invalid-line": { - content: `structured . -parameters`, - expectError: filename + ":1: not of the format : parameters", - }, - "invalid-check": { - content: `xxx .`, - expectError: filename + ":0: \"xxx\" is not a supported check: xxx .", - }, - } { - t.Run(name, func(t *testing.T) { - if err := ioutil.WriteFile(filename, []byte(tc.content), 0666); err != nil { - t.Fatalf("writing file: %v", err) - } - - filter := &RegexpFilter{ - validChecks: map[string]bool{ - structuredCheck: true, - parametersCheck: true, - }, - } - err := filter.Set(filename) - if tc.expectError == "" { - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - } else { - if err == nil { - t.Fatalf("did not get expected error: %s", tc.expectError) - } - if err.Error() != tc.expectError { - t.Fatalf("error mismatch\nexpected: %q\n got: %q", tc.expectError, err.Error()) - } - } - }) - } -} diff --git a/hack/tools/logcheck/pkg/logcheck.go b/hack/tools/logcheck/pkg/logcheck.go deleted file mode 100644 index 38122de0e..000000000 --- a/hack/tools/logcheck/pkg/logcheck.go +++ /dev/null @@ -1,498 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package pkg - -import ( - "flag" - "fmt" - "go/ast" - "go/token" - "go/types" - "os" - "path" - "strconv" - "strings" - - "golang.org/x/exp/utf8string" - "golang.org/x/tools/go/analysis" -) - -const ( - structuredCheck = "structured" - parametersCheck = "parameters" - contextualCheck = "contextual" - withHelpersCheck = "with-helpers" -) - -type checks map[string]*bool - -type config struct { - enabled checks - fileOverrides RegexpFilter -} - -func (c config) isEnabled(check string, filename string) bool { - return c.fileOverrides.Enabled(check, *c.enabled[check], filename) -} - -// Analyser creates a new logcheck analyser. -func Analyser() *analysis.Analyzer { - c := config{ - enabled: checks{ - structuredCheck: new(bool), - parametersCheck: new(bool), - contextualCheck: new(bool), - withHelpersCheck: new(bool), - }, - } - c.fileOverrides.validChecks = map[string]bool{} - for key := range c.enabled { - c.fileOverrides.validChecks[key] = true - } - logcheckFlags := flag.NewFlagSet("", flag.ExitOnError) - prefix := "check-" - logcheckFlags.BoolVar(c.enabled[structuredCheck], prefix+structuredCheck, true, `When true, logcheck will warn about calls to unstructured -klog methods (Info, Infof, Error, Errorf, Warningf, etc).`) - logcheckFlags.BoolVar(c.enabled[parametersCheck], prefix+parametersCheck, true, `When true, logcheck will check parameters of structured logging calls.`) - logcheckFlags.BoolVar(c.enabled[contextualCheck], prefix+contextualCheck, false, `When true, logcheck will only allow log calls for contextual logging (retrieving a Logger from klog or the context and logging through that) and warn about all others.`) - logcheckFlags.BoolVar(c.enabled[withHelpersCheck], prefix+withHelpersCheck, false, `When true, logcheck will warn about direct calls to WithName, WithValues and NewContext.`) - logcheckFlags.Var(&c.fileOverrides, "config", `A file which overrides the global settings for checks on a per-file basis via regular expressions.`) - - // Use env variables as defaults. This is necessary when used as plugin - // for golangci-lint because of - // https://github.com/golangci/golangci-lint/issues/1512. - for key, enabled := range c.enabled { - envVarName := "LOGCHECK_" + strings.ToUpper(strings.ReplaceAll(string(key), "-", "_")) - if value, ok := os.LookupEnv(envVarName); ok { - v, err := strconv.ParseBool(value) - if err != nil { - panic(fmt.Errorf("%s=%q: %v", envVarName, value, err)) - } - *enabled = v - } - } - if value, ok := os.LookupEnv("LOGCHECK_CONFIG"); ok { - if err := c.fileOverrides.Set(value); err != nil { - panic(fmt.Errorf("LOGCHECK_CONFIG=%q: %v", value, err)) - } - } - - return &analysis.Analyzer{ - Name: "logcheck", - Doc: "Tool to check logging calls.", - Run: func(pass *analysis.Pass) (interface{}, error) { - return run(pass, &c) - }, - Flags: *logcheckFlags, - } -} - -func run(pass *analysis.Pass, c *config) (interface{}, error) { - for _, file := range pass.Files { - ast.Inspect(file, func(n ast.Node) bool { - switch n := n.(type) { - case *ast.CallExpr: - // We are intrested in function calls, as we want to detect klog.* calls - // passing all function calls to checkForFunctionExpr - checkForFunctionExpr(n, pass, c) - case *ast.FuncType: - checkForContextAndLogger(n, n.Params, pass, c) - case *ast.IfStmt: - checkForIfEnabled(n, pass, c) - } - - return true - }) - } - return nil, nil -} - -// checkForFunctionExpr checks for unstructured logging function, prints error if found any. -func checkForFunctionExpr(fexpr *ast.CallExpr, pass *analysis.Pass, c *config) { - fun := fexpr.Fun - args := fexpr.Args - - /* we are extracting external package function calls e.g. klog.Infof fmt.Printf - and eliminating calls like setLocalHost() - basically function calls that has selector expression like . - */ - if selExpr, ok := fun.(*ast.SelectorExpr); ok { - // extracting function Name like Infof - fName := selExpr.Sel.Name - - filename := pass.Pkg.Path() + "/" + path.Base(pass.Fset.Position(fexpr.Pos()).Filename) - - // Now we need to determine whether it is coming from klog. - if isKlog(selExpr.X, pass) { - if c.isEnabled(contextualCheck, filename) && !isContextualCall(fName) { - pass.Report(analysis.Diagnostic{ - Pos: fun.Pos(), - Message: fmt.Sprintf("function %q should not be used, convert to contextual logging", fName), - }) - return - } - - // Matching if any unstructured logging function is used. - if c.isEnabled(structuredCheck, filename) && isUnstructured(fName) { - pass.Report(analysis.Diagnostic{ - Pos: fun.Pos(), - Message: fmt.Sprintf("unstructured logging function %q should not be used", fName), - }) - return - } - - if c.isEnabled(parametersCheck, filename) { - // if format specifier is used, check for arg length will most probably fail - // so check for format specifier first and skip if found - if checkForFormatSpecifier(fexpr, pass) { - return - } - if fName == "InfoS" { - isKeysValid(args[1:], fun, pass, fName) - } else if fName == "ErrorS" { - isKeysValid(args[2:], fun, pass, fName) - } - - // Also check structured calls. - if c.isEnabled(parametersCheck, filename) { - checkForFormatSpecifier(fexpr, pass) - } - } - } else if isGoLogger(selExpr.X, pass) { - if c.isEnabled(parametersCheck, filename) { - checkForFormatSpecifier(fexpr, pass) - switch fName { - case "WithValues": - isKeysValid(args, fun, pass, fName) - case "Info": - isKeysValid(args[1:], fun, pass, fName) - case "Error": - isKeysValid(args[2:], fun, pass, fName) - } - } - if c.isEnabled(withHelpersCheck, filename) { - switch fName { - case "WithValues", "WithName": - pass.Report(analysis.Diagnostic{ - Pos: fun.Pos(), - Message: fmt.Sprintf("function %q should be called through klogr.Logger%s", fName, fName), - }) - } - } - } else if fName == "NewContext" && - isPackage(selExpr.X, "github.com/go-logr/logr", pass) && - c.isEnabled(withHelpersCheck, filename) { - pass.Report(analysis.Diagnostic{ - Pos: fun.Pos(), - Message: fmt.Sprintf("function %q should be called through klogr.NewContext", fName), - }) - } - - } -} - -// isKlogVerbose returns true if the type of the expression is klog.Verbose (= -// the result of klog.V). -func isKlogVerbose(expr ast.Expr, pass *analysis.Pass) bool { - if typeAndValue, ok := pass.TypesInfo.Types[expr]; ok { - switch t := typeAndValue.Type.(type) { - case *types.Named: - if typeName := t.Obj(); typeName != nil { - if pkg := typeName.Pkg(); pkg != nil { - if typeName.Name() == "Verbose" && pkg.Path() == "k8s.io/klog/v2" { - return true - } - } - } - } - } - return false -} - -// isKlog checks whether an expression is klog.Verbose or the klog package itself. -func isKlog(expr ast.Expr, pass *analysis.Pass) bool { - // For klog.V(1) and klogV := klog.V(1) we can decide based on the type. - if isKlogVerbose(expr, pass) { - return true - } - - // In "klog.Info", "klog" is a package identifier. It doesn't need to - // be "klog" because here we look up the actual package. - return isPackage(expr, "k8s.io/klog/v2", pass) -} - -// isPackage checks whether an expression is an identifier that refers -// to a specific package like k8s.io/klog/v2. -func isPackage(expr ast.Expr, packagePath string, pass *analysis.Pass) bool { - if ident, ok := expr.(*ast.Ident); ok { - if object, ok := pass.TypesInfo.Uses[ident]; ok { - switch object := object.(type) { - case *types.PkgName: - pkg := object.Imported() - if pkg.Path() == packagePath { - return true - } - } - } - } - - return false -} - -// isGoLogger checks whether an expression is logr.Logger. -func isGoLogger(expr ast.Expr, pass *analysis.Pass) bool { - if typeAndValue, ok := pass.TypesInfo.Types[expr]; ok { - switch t := typeAndValue.Type.(type) { - case *types.Named: - if typeName := t.Obj(); typeName != nil { - if pkg := typeName.Pkg(); pkg != nil { - if typeName.Name() == "Logger" && pkg.Path() == "github.com/go-logr/logr" { - return true - } - } - } - } - } - return false -} - -func isUnstructured(fName string) bool { - // List of klog functions we do not want to use after migration to structured logging. - unstrucured := []string{ - "Infof", "Info", "Infoln", "InfoDepth", - "Warning", "Warningf", "Warningln", "WarningDepth", - "Error", "Errorf", "Errorln", "ErrorDepth", - "Fatal", "Fatalf", "Fatalln", "FatalDepth", - "Exit", "Exitf", "Exitln", "ExitDepth", - } - - for _, name := range unstrucured { - if fName == name { - return true - } - } - - return false -} - -func isContextualCall(fName string) bool { - // List of klog functions we still want to use after migration to - // contextual logging. This is an allow list, so any new acceptable - // klog call has to be added here. - contextual := []string{ - "Background", - "ClearLogger", - "ContextualLogger", - "EnableContextualLogging", - "FlushAndExit", - "FlushLogger", - "FromContext", - "KObj", - "KObjs", - "KRef", - "LoggerWithName", - "LoggerWithValues", - "NewContext", - "SetLogger", - "SetLoggerWithOptions", - "StartFlushDaemon", - "StopFlushDaemon", - "TODO", - } - for _, name := range contextual { - if fName == name { - return true - } - } - - return false -} - -// isKeysValid check if all keys in keyAndValues is string type -func isKeysValid(keyValues []ast.Expr, fun ast.Expr, pass *analysis.Pass, funName string) { - if len(keyValues)%2 != 0 { - pass.Report(analysis.Diagnostic{ - Pos: fun.Pos(), - Message: fmt.Sprintf("Additional arguments to %s should always be Key Value pairs. Please check if there is any key or value missing.", funName), - }) - return - } - - for index, arg := range keyValues { - if index%2 != 0 { - continue - } - lit, ok := arg.(*ast.BasicLit) - if !ok { - pass.Report(analysis.Diagnostic{ - Pos: fun.Pos(), - Message: fmt.Sprintf("Key positional arguments are expected to be inlined constant strings. Please replace %v provided with string value.", arg), - }) - continue - } - if lit.Kind != token.STRING { - pass.Report(analysis.Diagnostic{ - Pos: fun.Pos(), - Message: fmt.Sprintf("Key positional arguments are expected to be inlined constant strings. Please replace %v provided with string value.", lit.Value), - }) - continue - } - isASCII := utf8string.NewString(lit.Value).IsASCII() - if !isASCII { - pass.Report(analysis.Diagnostic{ - Pos: fun.Pos(), - Message: fmt.Sprintf("Key positional arguments %s are expected to be lowerCamelCase alphanumeric strings. Please remove any non-Latin characters.", lit.Value), - }) - } - } -} - -func checkForFormatSpecifier(expr *ast.CallExpr, pass *analysis.Pass) bool { - if selExpr, ok := expr.Fun.(*ast.SelectorExpr); ok { - // extracting function Name like Infof - fName := selExpr.Sel.Name - if strings.HasSuffix(fName, "f") { - // Allowed for calls like Infof. - return false - } - if specifier, found := hasFormatSpecifier(expr.Args); found { - msg := fmt.Sprintf("logging function %q should not use format specifier %q", fName, specifier) - pass.Report(analysis.Diagnostic{ - Pos: expr.Fun.Pos(), - Message: msg, - }) - return true - } - } - return false -} - -func hasFormatSpecifier(fArgs []ast.Expr) (string, bool) { - formatSpecifiers := []string{ - "%v", "%+v", "%#v", "%T", - "%t", "%b", "%c", "%d", "%o", "%O", "%q", "%x", "%X", "%U", - "%e", "%E", "%f", "%F", "%g", "%G", "%s", "%q", "%p", - } - for _, fArg := range fArgs { - if arg, ok := fArg.(*ast.BasicLit); ok { - for _, specifier := range formatSpecifiers { - if strings.Contains(arg.Value, specifier) { - return specifier, true - } - } - } - } - return "", false -} - -// checkForContextAndLogger ensures that a function doesn't accept both a -// context and a logger. That is problematic because it leads to ambiguity: -// does the context already contain the logger? That matters when passing it on -// without the logger. -func checkForContextAndLogger(n ast.Node, params *ast.FieldList, pass *analysis.Pass, c *config) { - var haveLogger, haveContext bool - - for _, param := range params.List { - if typeAndValue, ok := pass.TypesInfo.Types[param.Type]; ok { - switch t := typeAndValue.Type.(type) { - case *types.Named: - if typeName := t.Obj(); typeName != nil { - if pkg := typeName.Pkg(); pkg != nil { - if typeName.Name() == "Logger" && pkg.Path() == "github.com/go-logr/logr" { - haveLogger = true - } else if typeName.Name() == "Context" && pkg.Path() == "context" { - haveContext = true - } - } - } - } - } - } - - if haveLogger && haveContext { - pass.Report(analysis.Diagnostic{ - Pos: n.Pos(), - End: n.End(), - Message: `A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that.`, - }) - } -} - -// checkForIfEnabled detects `if klog.V(..).Enabled() { ...` and `if -// logger.V(...).Enabled()` and suggests capturing the result of V. -func checkForIfEnabled(i *ast.IfStmt, pass *analysis.Pass, c *config) { - // if i.Init == nil { - // A more complex if statement, let's assume it's okay. - // return - // } - - // Must be a method call. - callExpr, ok := i.Cond.(*ast.CallExpr) - if !ok { - return - } - selExpr, ok := callExpr.Fun.(*ast.SelectorExpr) - if !ok { - return - } - - // We only care about calls to Enabled(). - if selExpr.Sel.Name != "Enabled" { - return - } - - // And it must be Enabled for klog or logr.Logger. - if !isKlogVerbose(selExpr.X, pass) && - !isGoLogger(selExpr.X, pass) { - return - } - - // logger.Enabled() is okay, logger.V(1).Enabled() is not. - // That means we need to check for another selector expression - // with V as method name. - subCallExpr, ok := selExpr.X.(*ast.CallExpr) - if !ok { - return - } - subSelExpr, ok := subCallExpr.Fun.(*ast.SelectorExpr) - if !ok || subSelExpr.Sel.Name != "V" { - return - } - - // klogV is recommended as replacement for klog.V(). For logr.Logger - // let's use the root of the selector, which should be a variable. - varName := "klogV" - funcCall := "klog.V" - if isGoLogger(subSelExpr.X, pass) { - varName = "logger" - root := subSelExpr - for s, ok := root.X.(*ast.SelectorExpr); ok; s, ok = root.X.(*ast.SelectorExpr) { - root = s - } - if id, ok := root.X.(*ast.Ident); ok { - varName = id.Name - } - funcCall = varName + ".V" - } - - pass.Report(analysis.Diagnostic{ - Pos: i.Pos(), - End: i.End(), - Message: fmt.Sprintf("the result of %s should be stored in a variable and then be used multiple times: if %s := %s(); %s.Enabled() { ... %s.Info ... }", - funcCall, varName, funcCall, varName, varName), - }) -} diff --git a/hack/tools/logcheck/plugin/plugin.go b/hack/tools/logcheck/plugin/plugin.go deleted file mode 100644 index cd3e04ce9..000000000 --- a/hack/tools/logcheck/plugin/plugin.go +++ /dev/null @@ -1,35 +0,0 @@ -/* -Copyright 2022 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Package main is meant to be compiled as a plugin for golangci-lint, see -// https://golangci-lint.run/contributing/new-linters/#create-a-plugin. -package main - -import ( - "golang.org/x/tools/go/analysis" - "k8s.io/klog/hack/tools/logcheck/pkg" -) - -type analyzerPlugin struct{} - -func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer { - return []*analysis.Analyzer{ - pkg.Analyser(), - } -} - -// AnalyzerPlugin is the entry point for golangci-lint. -var AnalyzerPlugin analyzerPlugin diff --git a/hack/tools/logcheck/testdata/src/allowUnstructuredLogs/allowUnstructuredLogs.go b/hack/tools/logcheck/testdata/src/allowUnstructuredLogs/allowUnstructuredLogs.go deleted file mode 100644 index 045eb8d10..000000000 --- a/hack/tools/logcheck/testdata/src/allowUnstructuredLogs/allowUnstructuredLogs.go +++ /dev/null @@ -1,67 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// This fake package is created as golang.org/x/tools/go/analysis/analysistest -// expects it to be here for loading. This package is used to test allow-unstructured -// flag which suppresses errors when unstructured logging is used. -// This is a test file for unstructured logging static check tool unit tests. - -package allowUnstructuredLogs - -import ( - klog "k8s.io/klog/v2" -) - -func allowUnstructuredLogs() { - // Structured logs - // Error is expected if structured logging pattern is not used correctly - klog.InfoS("test log") - klog.ErrorS(nil, "test log") - klog.InfoS("Starting container in a pod", "containerID", "containerID", "pod") // want `Additional arguments to InfoS should always be Key Value pairs. Please check if there is any key or value missing.` - klog.ErrorS(nil, "Starting container in a pod", "containerID", "containerID", "pod") // want `Additional arguments to ErrorS should always be Key Value pairs. Please check if there is any key or value missing.` - klog.InfoS("Starting container in a pod", "测试", "containerID") // want `Key positional arguments "测试" are expected to be lowerCamelCase alphanumeric strings. Please remove any non-Latin characters.` - klog.ErrorS(nil, "Starting container in a pod", "测试", "containerID") // want `Key positional arguments "测试" are expected to be lowerCamelCase alphanumeric strings. Please remove any non-Latin characters.` - klog.InfoS("Starting container in a pod", 7, "containerID") // want `Key positional arguments are expected to be inlined constant strings. Please replace 7 provided with string value` - klog.ErrorS(nil, "Starting container in a pod", 7, "containerID") // want `Key positional arguments are expected to be inlined constant strings. Please replace 7 provided with string value` - klog.InfoS("Starting container in a pod", map[string]string{"test1": "value"}, "containerID") // want `Key positional arguments are expected to be inlined constant strings. ` - testKey := "a" - klog.ErrorS(nil, "Starting container in a pod", testKey, "containerID") // want `Key positional arguments are expected to be inlined constant strings. ` - klog.InfoS("test: %s", "testname") // want `logging function "InfoS" should not use format specifier "%s"` - klog.ErrorS(nil, "test no.: %d", 1) // want `logging function "ErrorS" should not use format specifier "%d"` - - // Unstructured logs - // Error is not expected as this package allows unstructured logging - klog.V(1).Infof("test log") - klog.Infof("test log") - klog.Info("test log") - klog.Infoln("test log") - klog.InfoDepth(1, "test log") - klog.Warning("test log") - klog.Warningf("test log") - klog.WarningDepth(1, "test log") - klog.Error("test log") - klog.Errorf("test log") - klog.Errorln("test log") - klog.ErrorDepth(1, "test log") - klog.Fatal("test log") - klog.Fatalf("test log") - klog.Fatalln("test log") - klog.FatalDepth(1, "test log") - klog.Exit("test log") - klog.ExitDepth(1, "test log") - klog.Exitln("test log") - klog.Exitf("test log") -} diff --git a/hack/tools/logcheck/testdata/src/contextual/contextual.go b/hack/tools/logcheck/testdata/src/contextual/contextual.go deleted file mode 100644 index 51053f6e6..000000000 --- a/hack/tools/logcheck/testdata/src/contextual/contextual.go +++ /dev/null @@ -1,49 +0,0 @@ -/* -Copyright 2022 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package contextual - -import ( - "context" - - "github.com/go-logr/logr" -) - -type myFuncType func(ctx context.Context, logger logr.Logger, msg string) // want `A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that.` - -func usingMyFuncType(firstParam int, - callback myFuncType, // Will be warned about at the type definition, not here. - lastParam int) { -} - -func usingInlineFunc(firstParam int, - callback func(ctx context.Context, logger logr.Logger, msg string), // want `A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that.` - lastParam int) { -} - -type myStruct struct { - myFuncField func(ctx context.Context, logger logr.Logger, msg string) // want `A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that.` -} - -func (m myStruct) myMethod(ctx context.Context, logger logr.Logger, msg string) { // want `A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that.` -} - -func myFunction(ctx context.Context, logger logr.Logger, msg string) { // want `A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that.` -} - -type myInterface interface { - myInterfaceMethod(ctx context.Context, logger logr.Logger, msg string) // want `A function should accept either a context or a logger, but not both. Having both makes calling the function harder because it must be defined whether the context must contain the logger and callers have to follow that.` -} diff --git a/hack/tools/logcheck/testdata/src/doNotAllowUnstructuredLogs/doNotAllowUnstructuredLogs.go b/hack/tools/logcheck/testdata/src/doNotAllowUnstructuredLogs/doNotAllowUnstructuredLogs.go deleted file mode 100644 index f03e0e989..000000000 --- a/hack/tools/logcheck/testdata/src/doNotAllowUnstructuredLogs/doNotAllowUnstructuredLogs.go +++ /dev/null @@ -1,67 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// This fake package is created as golang.org/x/tools/go/analysis/analysistest -// expects it to be here for loading. This package is used to test default -// behavior which is to report errors when unstructured logging is used. -// This is a test file for unstructured logging static check tool unit tests. - -package doNotAllowUnstructuredLogs - -import ( - klog "k8s.io/klog/v2" -) - -func doNotAllowUnstructuredLogs() { - // Structured logs - // Error is expected if structured logging pattern is not used correctly - klog.InfoS("test log") - klog.ErrorS(nil, "test log") - klog.InfoS("Starting container in a pod", "containerID", "containerID", "pod") // want `Additional arguments to InfoS should always be Key Value pairs. Please check if there is any key or value missing.` - klog.ErrorS(nil, "Starting container in a pod", "containerID", "containerID", "pod") // want `Additional arguments to ErrorS should always be Key Value pairs. Please check if there is any key or value missing.` - klog.InfoS("Starting container in a pod", "测试", "containerID") // want `Key positional arguments "测试" are expected to be lowerCamelCase alphanumeric strings. Please remove any non-Latin characters.` - klog.ErrorS(nil, "Starting container in a pod", "测试", "containerID") // want `Key positional arguments "测试" are expected to be lowerCamelCase alphanumeric strings. Please remove any non-Latin characters.` - klog.InfoS("Starting container in a pod", 7, "containerID") // want `Key positional arguments are expected to be inlined constant strings. Please replace 7 provided with string value` - klog.ErrorS(nil, "Starting container in a pod", 7, "containerID") // want `Key positional arguments are expected to be inlined constant strings. Please replace 7 provided with string value` - klog.InfoS("Starting container in a pod", map[string]string{"test1": "value"}, "containerID") // want `Key positional arguments are expected to be inlined constant strings. ` - testKey := "a" - klog.ErrorS(nil, "Starting container in a pod", testKey, "containerID") // want `Key positional arguments are expected to be inlined constant strings. ` - klog.InfoS("test: %s", "testname") // want `logging function "InfoS" should not use format specifier "%s"` - klog.ErrorS(nil, "test no.: %d", 1) // want `logging function "ErrorS" should not use format specifier "%d"` - - // Unstructured logs - // Error is expected as this package does not allow unstructured logging - klog.V(1).Infof("test log") // want `unstructured logging function "Infof" should not be used` - klog.Infof("test log") // want `unstructured logging function "Infof" should not be used` - klog.Info("test log") // want `unstructured logging function "Info" should not be used` - klog.Infoln("test log") // want `unstructured logging function "Infoln" should not be used` - klog.InfoDepth(1, "test log") // want `unstructured logging function "InfoDepth" should not be used` - klog.Warning("test log") // want `unstructured logging function "Warning" should not be used` - klog.Warningf("test log") // want `unstructured logging function "Warningf" should not be used` - klog.WarningDepth(1, "test log") // want `unstructured logging function "WarningDepth" should not be used` - klog.Error("test log") // want `unstructured logging function "Error" should not be used` - klog.Errorf("test log") // want `unstructured logging function "Errorf" should not be used` - klog.Errorln("test log") // want `unstructured logging function "Errorln" should not be used` - klog.ErrorDepth(1, "test log") // want `unstructured logging function "ErrorDepth" should not be used` - klog.Fatal("test log") // want `unstructured logging function "Fatal" should not be used` - klog.Fatalf("test log") // want `unstructured logging function "Fatalf" should not be used` - klog.Fatalln("test log") // want `unstructured logging function "Fatalln" should not be used` - klog.FatalDepth(1, "test log") // want `unstructured logging function "FatalDepth" should not be used` - klog.Exit("test log") // want `unstructured logging function "Exit" should not be used` - klog.Exitf("test log") // want `unstructured logging function "Exitf" should not be used` - klog.Exitln("test log") // want `unstructured logging function "Exitln" should not be used` - klog.ExitDepth(1, "test log") // want `unstructured logging function "ExitDepth" should not be used` -} diff --git a/hack/tools/logcheck/testdata/src/github.com/go-logr/logr/logr.go b/hack/tools/logcheck/testdata/src/github.com/go-logr/logr/logr.go deleted file mode 100644 index fa2da1e9d..000000000 --- a/hack/tools/logcheck/testdata/src/github.com/go-logr/logr/logr.go +++ /dev/null @@ -1,36 +0,0 @@ -/* -Copyright 2022 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Package logr provides empty stubs for github.com/go-logr/logr for testing -// with golang.org/x/tools/go/analysis/analysistest. -package logr - -import ( - "context" -) - -type Logger struct{} - -func (l Logger) Enabled() bool { return false } -func (l Logger) WithName(name string) Logger { return l } -func (l Logger) WithValues(kv ...interface{}) Logger { return l } -func (l Logger) V(level int) Logger { return l } -func (l Logger) Info(msg string, kv ...interface{}) {} -func (l Logger) Error(err error, msg string, kv ...interface{}) {} - -func NewContext(ctx context.Context, logger Logger) context.Context { - return nil -} diff --git a/hack/tools/logcheck/testdata/src/gologr/gologr.go b/hack/tools/logcheck/testdata/src/gologr/gologr.go deleted file mode 100644 index 6d533bb1b..000000000 --- a/hack/tools/logcheck/testdata/src/gologr/gologr.go +++ /dev/null @@ -1,38 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// This fake package is created as golang.org/x/tools/go/analysis/analysistest -// expects it to be here for loading. This package is used to test allow-unstructured -// flag which suppresses errors when unstructured logging is used. -// This is a test file for unstructured logging static check tool unit tests. - -package gologr - -import ( - "github.com/go-logr/logr" -) - -var logger logr.Logger - -func logging() { - logger.Info("hello", "missing value") // want `Additional arguments to Info should always be Key Value pairs. Please check if there is any key or value missing.` - logger.Error(nil, "hello", 1, 2) // want `Key positional arguments are expected to be inlined constant strings. Please replace 1 provided with string value` - logger.WithValues("missing value") // want `Additional arguments to WithValues should always be Key Value pairs. Please check if there is any key or value missing.` - - logger.V(1).Info("hello", "missing value") // want `Additional arguments to Info should always be Key Value pairs. Please check if there is any key or value missing.` - logger.V(1).Error(nil, "hello", 1, 2) // want `Key positional arguments are expected to be inlined constant strings. Please replace 1 provided with string value` - logger.V(1).WithValues("missing value") // want `Additional arguments to WithValues should always be Key Value pairs. Please check if there is any key or value missing.` -} diff --git a/hack/tools/logcheck/testdata/src/helpers/doNotAllowDirectCalls.go b/hack/tools/logcheck/testdata/src/helpers/doNotAllowDirectCalls.go deleted file mode 100644 index 22d8bb84d..000000000 --- a/hack/tools/logcheck/testdata/src/helpers/doNotAllowDirectCalls.go +++ /dev/null @@ -1,43 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// This fake package is created as golang.org/x/tools/go/analysis/analysistest -// expects it to be here for loading. This package is used to test allow-unstructured -// flag which suppresses errors when unstructured logging is used. -// This is a test file for unstructured logging static check tool unit tests. - -package helpers - -import ( - "context" - - "github.com/go-logr/logr" - klog "k8s.io/klog/v2" -) - -var logger klog.Logger - -func doNotAlllowDirectCalls() { - logger.WithName("foo") // want `function "WithName" should be called through klogr.LoggerWithName` - logger.WithValues("a", "b") // want `function "WithValues" should be called through klogr.LoggerWithValues` - logr.NewContext(context.Background(), logger) // want `function "NewContext" should be called through klogr.NewContext` -} - -func allowHelpers() { - klog.LoggerWithName(logger, "foo") - klog.LoggerWithValues(logger, "a", "b") - klog.NewContext(context.Background(), logger) -} diff --git a/hack/tools/logcheck/testdata/src/importrename/importrename.go b/hack/tools/logcheck/testdata/src/importrename/importrename.go deleted file mode 100644 index b5382bc60..000000000 --- a/hack/tools/logcheck/testdata/src/importrename/importrename.go +++ /dev/null @@ -1,30 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// This fake package is created as golang.org/x/tools/go/analysis/analysistest -// expects it to be here for loading. This package is used to test allow-unstructured -// flag which suppresses errors when unstructured logging is used. -// This is a test file for unstructured logging static check tool unit tests. - -package importrename - -import ( - glog "k8s.io/klog/v2" -) - -func dontAllowUnstructuredLogs() { - glog.Info("test log") // want `unstructured logging function "Info" should not be used` -} diff --git a/hack/tools/logcheck/testdata/src/k8s.io/klog/v2/klog.go b/hack/tools/logcheck/testdata/src/k8s.io/klog/v2/klog.go deleted file mode 100644 index d62a5d433..000000000 --- a/hack/tools/logcheck/testdata/src/k8s.io/klog/v2/klog.go +++ /dev/null @@ -1,250 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// This fake package is created as package golang.org/x/tools/go/analysis/analysistest -// expects test data dependency to be testdata/src - -package v2 - -import ( - "context" - - "github.com/go-logr/logr" -) - -// Verbose is a boolean type that implements Infof (like Printf) etc. -// See the documentation of V for more information. -type Verbose struct { - enabled bool -} - -type Level int32 - -type Logger = logr.Logger - -func V(level Level) Verbose { - - return Verbose{enabled: false} -} - -// Enabled returns true if logging at the selected level is enabled. -func (v Verbose) Enabled() bool { - return false -} - -// Info is equivalent to the global Info function, guarded by the value of v. -// See the documentation of V for usage. -func (v Verbose) Info(args ...interface{}) { - -} - -// Infoln is equivalent to the global Infoln function, guarded by the value of v. -// See the documentation of V for usage. -func (v Verbose) Infoln(args ...interface{}) { - -} - -// Infof is equivalent to the global Infof function, guarded by the value of v. -// See the documentation of V for usage. -func (v Verbose) Infof(format string, args ...interface{}) { - -} - -// InfoS is equivalent to the global InfoS function, guarded by the value of v. -// See the documentation of V for usage. -func (v Verbose) InfoS(msg string, keysAndValues ...interface{}) { - -} - -func InfoSDepth(depth int, msg string, keysAndValues ...interface{}) { -} - -// Deprecated: Use ErrorS instead. -func (v Verbose) Error(err error, msg string, args ...interface{}) { - -} - -// ErrorS is equivalent to the global Error function, guarded by the value of v. -// See the documentation of V for usage. -func (v Verbose) ErrorS(err error, msg string, keysAndValues ...interface{}) { - -} - -// Info logs to the INFO log. -// Arguments are handled in the manner of fmt.Print; a newline is appended if missing. -func Info(args ...interface{}) { -} - -// InfoDepth acts as Info but uses depth to determine which call frame to log. -// InfoDepth(0, "msg") is the same as Info("msg"). -func InfoDepth(depth int, args ...interface{}) { -} - -// Infoln logs to the INFO log. -// Arguments are handled in the manner of fmt.Println; a newline is always appended. -func Infoln(args ...interface{}) { -} - -// Infof logs to the INFO log. -// Arguments are handled in the manner of fmt.Printf; a newline is appended if missing. -func Infof(format string, args ...interface{}) { -} - -// InfoS structured logs to the INFO log. -// The msg argument used to add constant description to the log line. -// The key/value pairs would be join by "=" ; a newline is always appended. -// -// Basic examples: -// >> klog.InfoS("Pod status updated", "pod", "kubedns", "status", "ready") -// output: -// >> I1025 00:15:15.525108 1 controller_utils.go:116] "Pod status updated" pod="kubedns" status="ready" -func InfoS(msg string, keysAndValues ...interface{}) { -} - -// Warning logs to the WARNING and INFO logs. -// Arguments are handled in the manner of fmt.Print; a newline is appended if missing. -func Warning(args ...interface{}) { -} - -// WarningDepth acts as Warning but uses depth to determine which call frame to log. -// WarningDepth(0, "msg") is the same as Warning("msg"). -func WarningDepth(depth int, args ...interface{}) { -} - -// Warningln logs to the WARNING and INFO logs. -// Arguments are handled in the manner of fmt.Println; a newline is always appended. -func Warningln(args ...interface{}) { -} - -// Warningf logs to the WARNING and INFO logs. -// Arguments are handled in the manner of fmt.Printf; a newline is appended if missing. -func Warningf(format string, args ...interface{}) { -} - -// Error logs to the ERROR, WARNING, and INFO logs. -// Arguments are handled in the manner of fmt.Print; a newline is appended if missing. -func Error(args ...interface{}) { -} - -// ErrorDepth acts as Error but uses depth to determine which call frame to log. -// ErrorDepth(0, "msg") is the same as Error("msg"). -func ErrorDepth(depth int, args ...interface{}) { -} - -// Errorln logs to the ERROR, WARNING, and INFO logs. -// Arguments are handled in the manner of fmt.Println; a newline is always appended. -func Errorln(args ...interface{}) { -} - -// Errorf logs to the ERROR, WARNING, and INFO logs. -// Arguments are handled in the manner of fmt.Printf; a newline is appended if missing. -func Errorf(format string, args ...interface{}) { -} - -// ErrorS structured logs to the ERROR, WARNING, and INFO logs. -// the err argument used as "err" field of log line. -// The msg argument used to add constant description to the log line. -// The key/value pairs would be join by "=" ; a newline is always appended. -// -// Basic examples: -// >> klog.ErrorS(err, "Failed to update pod status") -// output: -// >> E1025 00:15:15.525108 1 controller_utils.go:114] "Failed to update pod status" err="timeout" -func ErrorS(err error, msg string, keysAndValues ...interface{}) { -} - -// ErrorSDepth acts as ErrorS but uses depth to determine which call frame to log. -// ErrorSDepth(0, "msg") is the same as ErrorS("msg"). -func ErrorSDepth(depth int, err error, msg string, keysAndValues ...interface{}) { -} - -// Fatal logs to the FATAL, ERROR, WARNING, and INFO logs, -// including a stack trace of all running goroutines, then calls os.Exit(255). -// Arguments are handled in the manner of fmt.Print; a newline is appended if missing. -func Fatal(args ...interface{}) { -} - -// FatalDepth acts as Fatal but uses depth to determine which call frame to log. -// FatalDepth(0, "msg") is the same as Fatal("msg"). -func FatalDepth(depth int, args ...interface{}) { -} - -// Fatalln logs to the FATAL, ERROR, WARNING, and INFO logs, -// including a stack trace of all running goroutines, then calls os.Exit(255). -// Arguments are handled in the manner of fmt.Println; a newline is always appended. -func Fatalln(args ...interface{}) { -} - -// Fatalf logs to the FATAL, ERROR, WARNING, and INFO logs, -// including a stack trace of all running goroutines, then calls os.Exit(255). -// Arguments are handled in the manner of fmt.Printf; a newline is appended if missing. -func Fatalf(format string, args ...interface{}) { -} - -func Exit(args ...interface{}) { -} - -// ExitDepth acts as Exit but uses depth to determine which call frame to log. -// ExitDepth(0, "msg") is the same as Exit("msg"). -func ExitDepth(depth int, args ...interface{}) { -} - -// Exitln logs to the FATAL, ERROR, WARNING, and INFO logs, then calls os.Exit(1). -func Exitln(args ...interface{}) { -} - -// Exitf logs to the FATAL, ERROR, WARNING, and INFO logs, then calls os.Exit(1). -// Arguments are handled in the manner of fmt.Printf; a newline is appended if missing. -func Exitf(format string, args ...interface{}) { -} - -// KObj emulates klog.KObj -func KObj(obj interface{}) interface{} { - return nil -} - -// KObjs emulates klog.KObjs -func KObjs(obj interface{}) interface{} { - return nil -} - -func KRef(namespace, name string) interface{} { - return nil -} - -func FromContext(ctx context.Context) Logger { - return Logger{} -} - -func NewContext(ctx context.Context, logger Logger) context.Context { - return ctx -} - -func LoggerWithName(logger Logger, name string) Logger { - return Logger{} -} - -func LoggerWithValues(logger Logger, kvs ...interface{}) Logger { - return Logger{} -} - -func TODO() Logger { - return Logger{} -} - -func Background() Logger { - return Logger{} -} diff --git a/hack/tools/logcheck/testdata/src/mixed/allowUnstructuredLogs.go b/hack/tools/logcheck/testdata/src/mixed/allowUnstructuredLogs.go deleted file mode 100644 index 6215b2476..000000000 --- a/hack/tools/logcheck/testdata/src/mixed/allowUnstructuredLogs.go +++ /dev/null @@ -1,31 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// This fake package is created as golang.org/x/tools/go/analysis/analysistest -// expects it to be here for loading. This package is used to test allow-unstructured -// flag which suppresses errors when unstructured logging is used. -// This is a test file for unstructured logging static check tool unit tests. - -package mixed - -import ( - klog "k8s.io/klog/v2" -) - -func allowUnstructuredLogs() { - // Error is not expected as this file allows unstructured logging - klog.Infof("test log") -} diff --git a/hack/tools/logcheck/testdata/src/mixed/doNotAllowUnstructuredLogs.go b/hack/tools/logcheck/testdata/src/mixed/doNotAllowUnstructuredLogs.go deleted file mode 100644 index bd448e97c..000000000 --- a/hack/tools/logcheck/testdata/src/mixed/doNotAllowUnstructuredLogs.go +++ /dev/null @@ -1,30 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// This fake package is created as golang.org/x/tools/go/analysis/analysistest -// expects it to be here for loading. This package is used to test allow-unstructured -// flag which suppresses errors when unstructured logging is used. -// This is a test file for unstructured logging static check tool unit tests. - -package mixed - -import ( - klog "k8s.io/klog/v2" -) - -func dontAllowUnstructuredLogs() { - klog.Info("test log") // want `unstructured logging function "Info" should not be used` -} diff --git a/hack/tools/logcheck/testdata/src/mixed/structured_logging b/hack/tools/logcheck/testdata/src/mixed/structured_logging deleted file mode 100644 index 050a12714..000000000 --- a/hack/tools/logcheck/testdata/src/mixed/structured_logging +++ /dev/null @@ -1,6 +0,0 @@ -# This file contains regular expressions that are matched against /, -# for example k8s.io/cmd/kube-scheduler/app/config/config.go. -# -# Any file that is matched may only use structured logging calls. - -structured .*doNotAllowUnstructuredLogs.go diff --git a/hack/tools/logcheck/testdata/src/onlyAllowContextual/klog_logging b/hack/tools/logcheck/testdata/src/onlyAllowContextual/klog_logging deleted file mode 100644 index d298e40aa..000000000 --- a/hack/tools/logcheck/testdata/src/onlyAllowContextual/klog_logging +++ /dev/null @@ -1 +0,0 @@ -contextual .*onlyAllowContextual.go diff --git a/hack/tools/logcheck/testdata/src/onlyAllowContextual/onlyAllowContextual.go b/hack/tools/logcheck/testdata/src/onlyAllowContextual/onlyAllowContextual.go deleted file mode 100644 index 4cc5e128e..000000000 --- a/hack/tools/logcheck/testdata/src/onlyAllowContextual/onlyAllowContextual.go +++ /dev/null @@ -1,32 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// This fake package is created as golang.org/x/tools/go/analysis/analysistest -// expects it to be here for loading. This package is used to test allow-unstructured -// flag which suppresses errors when unstructured logging is used. -// This is a test file for unstructured logging static check tool unit tests. - -package onlyallowcontextual - -import ( - klog "k8s.io/klog/v2" -) - -func doNotAlllowKlog() { - klog.InfoS("test log") // want `function "InfoS" should not be used, convert to contextual logging` - klog.ErrorS(nil, "test log") // want `function "ErrorS" should not be used, convert to contextual logging` - klog.V(1).Infof("test log") // want `function "V" should not be used, convert to contextual logging` `function "Infof" should not be used, convert to contextual logging` -} diff --git a/hack/tools/logcheck/testdata/src/onlyAllowContextual/whitelistedKlog.go b/hack/tools/logcheck/testdata/src/onlyAllowContextual/whitelistedKlog.go deleted file mode 100644 index 07bb2a2a9..000000000 --- a/hack/tools/logcheck/testdata/src/onlyAllowContextual/whitelistedKlog.go +++ /dev/null @@ -1,37 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// This fake package is created as golang.org/x/tools/go/analysis/analysistest -// expects it to be here for loading. This package is used to test allow-unstructured -// flag which suppresses errors when unstructured logging is used. -// This is a test file for unstructured logging static check tool unit tests. - -package onlyallowcontextual - -import ( - klog "k8s.io/klog/v2" -) - -func allowKlog() { - klog.KObj(nil) - klog.KObjs(nil) - klog.KRef("", "") - klog.FromContext(nil) - klog.TODO() - klog.Background() - klog.LoggerWithName(klog.Logger{}, "foo") - klog.LoggerWithValues(klog.Logger{}, "a", "b") -} diff --git a/hack/tools/logcheck/testdata/src/parameters/parameters.go b/hack/tools/logcheck/testdata/src/parameters/parameters.go deleted file mode 100644 index 919870f1e..000000000 --- a/hack/tools/logcheck/testdata/src/parameters/parameters.go +++ /dev/null @@ -1,55 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// This fake package is created as golang.org/x/tools/go/analysis/analysistest -// expects it to be here for loading. This package is used to test parameter -// checking. - -package parameters - -import ( - "fmt" - - klog "k8s.io/klog/v2" -) - -func parameterCalls() { - // format strings (incomplete list...) - klog.Infof("%d", 1) - klog.InfoS("%d", 1) // want `logging function "InfoS" should not use format specifier "%d"` - klog.Info("%d", 1) // want `logging function "Info" should not use format specifier "%d"` - klog.Infoln("%d", 1) // want `logging function "Infoln" should not use format specifier "%d"` - klog.V(1).Infof("%d", 1) - klog.V(1).InfoS("%d", 1) // want `logging function "InfoS" should not use format specifier "%d"` - klog.V(1).Info("%d", 1) // want `logging function "Info" should not use format specifier "%d"` - klog.V(1).Infoln("%d", 1) // want `logging function "Infoln" should not use format specifier "%d"` - klog.Errorf("%d", 1) - klog.ErrorS(nil, "%d", 1) // want `logging function "ErrorS" should not use format specifier "%d"` - klog.Error("%d", 1) // want `logging function "Error" should not use format specifier "%d"` - klog.Errorln("%d", 1) // want `logging function "Errorln" should not use format specifier "%d"` - - klog.InfoS("hello", "value", fmt.Sprintf("%d", 1)) - - // odd number of parameters - klog.InfoS("hello", "key") // want `Additional arguments to InfoS should always be Key Value pairs. Please check if there is any key or value missing.` - klog.ErrorS(nil, "hello", "key") // want `Additional arguments to ErrorS should always be Key Value pairs. Please check if there is any key or value missing.` - - // non-string keys - klog.InfoS("hello", "1", 2) - klog.InfoS("hello", 1, 2) // want `Key positional arguments are expected to be inlined constant strings. Please replace 1 provided with string value` - klog.ErrorS(nil, "hello", "1", 2) - klog.ErrorS(nil, "hello", 1, 2) // want `Key positional arguments are expected to be inlined constant strings. Please replace 1 provided with string value` -} diff --git a/hack/tools/logcheck/testdata/src/verbose/verbose.go b/hack/tools/logcheck/testdata/src/verbose/verbose.go deleted file mode 100644 index fa8758c0b..000000000 --- a/hack/tools/logcheck/testdata/src/verbose/verbose.go +++ /dev/null @@ -1,57 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// This fake package is created as golang.org/x/tools/go/analysis/analysistest -// expects it to be here for loading. This package is used to test allow-unstructured -// flag which suppresses errors when unstructured logging is used. -// This is a test file for unstructured logging static check tool unit tests. - -package verbose - -import ( - "github.com/go-logr/logr" - klog "k8s.io/klog/v2" -) - -var l, logger logr.Logger - -func verboseLogging() { - klog.V(1).Info("test log") // want `unstructured logging function "Info" should not be used` - if klogV := klog.V(1); klogV.Enabled() { - klogV.Infof("hello %s", "world") // want `unstructured logging function "Infof" should not be used` - } - - // \(\) is actually () in the diagnostic output. We have to escape here - // because `want` expects a regular expression. - - if klog.V(1).Enabled() { // want `the result of klog.V should be stored in a variable and then be used multiple times: if klogV := klog.V\(\); klogV.Enabled\(\) { ... klogV.Info ... }` - klog.V(1).InfoS("I'm logging at level 1.") - } - - if l.V(1).Enabled() { // want `the result of l.V should be stored in a variable and then be used multiple times: if l := l.V\(\); l.Enabled\(\) { ... l.Info ... }` - l.V(1).Info("I'm logging at level 1.") - } - - if l := l.V(2); l.Enabled() { - l.Info("I'm logging at level 2.") - } - - if l := logger.V(2); l.Enabled() { - // This is probably an error (should be l instead of logger), - // but not currently detected. - logger.Info("I wanted to log at level 2, but really it is 0.") - } -}