Support documented-based @NOLINT directives (#20)

This mimics the annotation-based syntax:

    nolint = ""         @NOLINT
    nolint = "a,b"      @NOLINT(a,b)

The annotation syntax is preferred, but the documentation block syntax
is useful for those few cases where the target node doesn't support
Thrift annotations (such as `const` declarations).

README.md

@@ -167,14 +167,16 @@ additional checks.
 
 [ast-node]: https://pkg.go.dev/go.uber.org/thriftrw/ast#Node
 
-## `nolint` Annotations
+## `nolint` Directives
 
 You can disable one or more checks on a per-node basis using `nolint`
-annotations. `nolint` annotations apply to the current node and all of its
-descendents. The annotation's value can be empty, in which case linting is
+directives. `nolint` directives apply to the current node and all of its
+descendents. The directives's value can be empty, in which case linting is
 entirely disabled, or it can be set to a comma-separated list of checks to
 disable.
 
+`nolint` directives can be written as Thrift annotations:
+
 ```thrift
 enum State {
 	STOPPED = 1
@@ -184,6 +186,26 @@ enum State {
 } (nolint = "enum.size")
 ```
 
+... and as `@nolint` lines in documentation blocks:
+
+```thrift
+/**
+ * States
+ *
+ * @nolint(enum.size)
+ */
+enum State {
+	STOPPED = 1
+	RUNNING = 2
+	PASSED = 3
+	FAILED = 4
+}
+```
+
+The annotation syntax is preferred, but the documentation block syntax is
+useful for those few cases where the target node doesn't support Thrift
+annotations (such as `const` declarations).
+
 ## License
 
 This software is released under the terms of the [Apache 2.0 License](LICENSE).

ast.go

@@ -47,6 +47,16 @@ func Annotations(node ast.Node) []*ast.Annotation {
 	return nil
 }
 
+// Doc returns an ast.Node's Doc string.
+func Doc(node ast.Node) string {
+	if v := reflect.ValueOf(node); v.Kind() == reflect.Ptr {
+		if f := v.Elem().FieldByName("Doc"); f.IsValid() {
+			return f.Interface().(string)
+		}
+	}
+	return ""
+}
+
 // Resolve resolves an ast.TypeReference to its target node.
 //
 // The target can either be in the current program's scope or it can refer to

ast_test.go

@@ -27,8 +27,8 @@ func TestAnnotations(t *testing.T) {
 		{Name: "test2", Value: "value2"},
 	}
 	tests := []struct {
-		node     ast.Node
-		expected []*ast.Annotation
+		node ast.Node
+		want []*ast.Annotation
 	}{
 		{&ast.Struct{}, nil},
 		{&ast.Struct{Annotations: annotations}, annotations},
@@ -37,9 +37,27 @@ func TestAnnotations(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		actual := Annotations(tt.node)
-		if !reflect.DeepEqual(actual, tt.expected) {
-			t.Errorf("expected %s but got %s", tt.expected, actual)
+		got := Annotations(tt.node)
+		if !reflect.DeepEqual(got, tt.want) {
+			t.Errorf("expected %s but got %s", tt.want, got)
+		}
+	}
+}
+
+func TestDoc(t *testing.T) {
+	tests := []struct {
+		node ast.Node
+		want string
+	}{
+		{&ast.Struct{}, ""},
+		{&ast.Struct{Doc: ""}, ""},
+		{&ast.Struct{Doc: "String"}, "String"},
+	}
+
+	for _, tt := range tests {
+		got := Doc(tt.node)
+		if got != tt.want {
+			t.Errorf("expected %s but got %s", tt.want, got)
 		}
 	}
 }

linter.go

@@ -127,23 +127,13 @@ func (l *Linter) lint(program *ast.Program, filename string, parseInfo *idl.Info
 		nodes := append([]ast.Node{n}, w.Ancestors()...)
 		checks := *activeChecks.lookup(nodes[1:])
 
-		// Handle 'nolint' annotations
-		if annotations := Annotations(n); annotations != nil {
-			for _, annotation := range annotations {
-				if annotation.Name == "nolint" {
-					if annotation.Value == "" {
-						return nil
-					}
-
-					values := strings.Split(annotation.Value, ",")
-					for i := range values {
-						values[i] = strings.TrimSpace(values[i])
-					}
-
-					checks = checks.Without(values)
-					activeChecks.add(n, &checks)
-				}
+		// Handle 'nolint' directives.
+		if names, found := nolint(n); found {
+			if names == nil {
+				return nil
 			}
+			checks = checks.Without(names)
+			activeChecks.add(n, &checks)
 		}
 
 		// Run all of the checks that match this part of the tree.

nolint.go

@@ -0,0 +1,65 @@
+// Copyright 2021 Pinterest
+//
+// 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 thriftcheck
+
+import (
+	"regexp"
+	"strings"
+
+	"go.uber.org/thriftrw/ast"
+)
+
+var nolintRegexp = regexp.MustCompile(`@nolint(?:\((.*)\))?`)
+
+func nolint(n ast.Node) ([]string, bool) {
+	var names []string
+
+	if annotations := Annotations(n); annotations != nil {
+		for _, annotation := range annotations {
+			if annotation.Name == "nolint" {
+				if annotation.Value == "" {
+					return nil, true
+				}
+				names = append(names, splitTrim(annotation.Value, ",")...)
+			}
+		}
+	}
+
+	if doc := Doc(n); doc != "" {
+		if m := nolintRegexp.FindStringSubmatch(doc); len(m) == 2 {
+			if m[1] == "" {
+				return nil, true
+			}
+			names = append(names, splitTrim(m[1], ",")...)
+		}
+	}
+
+	if names == nil {
+		return nil, false
+	}
+
+	// This may contain duplicate names at this point, but given how this
+	// return value is used, we can tolerate that without doing the extra
+	// work here to de-duplicate.
+	return names, true
+}
+
+func splitTrim(s, sep string) []string {
+	values := strings.Split(s, sep)
+	for i := range values {
+		values[i] = strings.TrimSpace(values[i])
+	}
+	return values
+}

nolint_test.go

@@ -0,0 +1,112 @@
+// Copyright 2021 Pinterest
+//
+// 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 thriftcheck
+
+import (
+	"reflect"
+	"testing"
+
+	"go.uber.org/thriftrw/ast"
+)
+
+func TestNolint(t *testing.T) {
+	tests := []struct {
+		desc  string
+		node  ast.Node
+		ok    bool
+		names []string
+	}{
+		{
+			desc:  "empty",
+			node:  &ast.Struct{},
+			ok:    false,
+			names: nil,
+		},
+		{
+			desc:  "words",
+			node:  &ast.Struct{Doc: "Just some words"},
+			ok:    false,
+			names: nil,
+		},
+		{
+			desc: `(nolint = "")`,
+			node: &ast.Struct{
+				Annotations: []*ast.Annotation{{Name: "nolint", Value: ""}},
+			},
+			ok:    true,
+			names: nil,
+		},
+		{
+			desc: `(nolint = "a")`,
+			node: &ast.Struct{
+				Annotations: []*ast.Annotation{{Name: "nolint", Value: "a"}},
+			},
+			ok:    true,
+			names: []string{"a"},
+		},
+		{
+			desc: `(nolint = "a,b")`,
+			node: &ast.Struct{
+				Annotations: []*ast.Annotation{{Name: "nolint", Value: "a,b"}},
+			},
+			ok:    true,
+			names: []string{"a", "b"},
+		},
+		{
+			desc:  "@nolint",
+			node:  &ast.Struct{Doc: "@nolint"},
+			ok:    true,
+			names: nil,
+		},
+		{
+			desc:  "@nolint()",
+			node:  &ast.Struct{Doc: "@nolint()"},
+			ok:    true,
+			names: nil,
+		},
+		{
+			desc:  "@nolint(a)",
+			node:  &ast.Struct{Doc: "@nolint(a)"},
+			ok:    true,
+			names: []string{"a"},
+		},
+		{
+			desc:  "@nolint(a,b)",
+			node:  &ast.Struct{Doc: "@nolint(a,b)"},
+			ok:    true,
+			names: []string{"a", "b"},
+		},
+		{
+			desc: `(nolint = "a,b") and @nolint(b,c)`,
+			node: &ast.Struct{
+				Annotations: []*ast.Annotation{{Name: "nolint", Value: "a,b"}},
+				Doc:         "@nolint(b,c)",
+			},
+			ok:    true,
+			names: []string{"a", "b", "b", "c"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			names, ok := nolint(tt.node)
+			if ok != tt.ok {
+				t.Errorf("expected %v, got %v", tt.ok, ok)
+			} else if !reflect.DeepEqual(names, tt.names) {
+				t.Errorf("expected %v, got %v", tt.names, names)
+			}
+		})
+	}
+}