commit 8f1bd315f62d8e47768bc3ab5ceb57edbcf8158b Author: Alec Thomas Date: Wed Jan 11 15:36:58 2017 +1100 Initial export of Thrift linter. diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..daf913b --- /dev/null +++ b/.gitignore @@ -0,0 +1,24 @@ +# Compiled Object files, Static and Dynamic libs (Shared Objects) +*.o +*.a +*.so + +# Folders +_obj +_test + +# Architecture specific extensions/prefixes +*.[568vq] +[568vq].out + +*.cgo1.go +*.cgo2.c +_cgo_defun.c +_cgo_gotypes.go +_cgo_export.* + +_testmain.go + +*.exe +*.test +*.prof diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..5d13e37 --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2017 Compass + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md new file mode 100644 index 0000000..d2a8d2b --- /dev/null +++ b/README.md @@ -0,0 +1,58 @@ +# An extensible linter for Thrift [![](https://godoc.org/github.com/UrbanCompass/thriftlint?status.svg)](http://godoc.org/github.com/UrbanCompass/thriftlint) + +This is an extensible linter for [Thrift](https://thrift.apache.org/). It +includes a set of common lint checks, but allows for both customisation of those +checks, and creation of new ones by implementing the +[Check](https://godoc.org/github.com/UrbanCompass/thriftlint#Check) interface. + +For an example of how to build your own linter utility, please refer to the +[thrift-lint source](https://github.com/UrbanCompass/thriftlint/tree/master/cmd/thrift-lint). + +## Example checker + +Here is an example of a checker utilising the `MakeCheck()` convenience +function to ensure that fields are present in the file in the same order as +their field numbers: + +```go +func CheckStructFieldOrder() thriftlint.Check { + return thriftlint.MakeCheck("field.order", func(s *parser.Struct) (messages thriftlint.Messages) { + fields := sortedFields(s.Fields) + sort.Sort(fields) + for i := 0; i < len(fields)-1; i++ { + a := fields[i] + b := fields[i+1] + if a.Pos.Line > b.Pos.Line { + messages.Warning(fields[i], "field %d and %d are out of order", a.ID, b.ID) + } + } + return + }) +} +``` + +## thrift-lint tool + +A binary is included that can be used to perform basic linting with the builtin checks: + +``` +$ go get github.com/UrbanCompass/thriftlint/cmd/thrift-lint +$ thrift-lint --help +usage: thrift-lint [] ... + +A linter for Thrift. + +For details, please refer to https://github.com/UrbanCompass/thriftlint + +Flags: + --help Show context-sensitive help (also try --help-long + and --help-man). + -I, --include=DIR ... Include directories to search. + --debug Enable debug logging. + --disable=LINTER ... Linters to disable. + --list List linter checks. + --errors Only show errors. + +Args: + Thrift sources to lint. +``` diff --git a/annotations.go b/annotations.go new file mode 100644 index 0000000..50f8cc2 --- /dev/null +++ b/annotations.go @@ -0,0 +1,28 @@ +package thriftlint + +import ( + "reflect" + + "github.com/alecthomas/go-thrift/parser" +) + +// Annotation returns the annotation value associated with "key" from the .Annotations field of a +// go-thrift AST node. +// +// This will panic if node is not a struct with an Annotations field of the correct type. +func Annotation(node interface{}, key, dflt string) string { + annotations := reflect.Indirect(reflect.ValueOf(node)). + FieldByName("Annotations"). + Interface().([]*parser.Annotation) + for _, annotation := range annotations { + if annotation.Name == key { + return annotation.Value + } + } + return dflt +} + +// AnnotationExists checks if an annotation is present at all. +func AnnotationExists(node interface{}, key string) bool { + return Annotation(node, key, "\000") != "\000" +} diff --git a/api.go b/api.go new file mode 100644 index 0000000..764e3ee --- /dev/null +++ b/api.go @@ -0,0 +1,130 @@ +package thriftlint + +import ( + "fmt" + "strings" + + "github.com/alecthomas/go-thrift/parser" +) + +// Severity of a linter message. +type Severity int + +// Message severities. +const ( + Warning Severity = iota + Error +) + +func (s Severity) String() string { + if s == Warning { + return "warning" + } + return "error" +} + +// Message represents a single linter message. +type Message struct { + // File that resulted in the message. + File *parser.Thrift + // ID of the Checker that generated this message. + Checker string + Severity Severity + Object interface{} + Message string +} + +// Messages is the set of messages each check should return. +// +// Typically it will be used like so: +// +// func MyCheck(...) (messages Messages) { +// messages.Warning(t, "some warning") +// } +type Messages []*Message + +// Warning adds a warning-level message to the Messages. +func (w *Messages) Warning(object interface{}, msg string, args ...interface{}) Messages { + message := &Message{Severity: Warning, Object: object, Message: fmt.Sprintf(msg, args...)} + *w = append(*w, message) + return *w +} + +// Warning adds an error-level message to the Messages. +func (w *Messages) Error(object interface{}, msg string, args ...interface{}) Messages { + message := &Message{Severity: Error, Object: object, Message: fmt.Sprintf(msg, args...)} + *w = append(*w, message) + return *w +} + +// Checks is a convenience wrapper around a slice of Checks. +type Checks []Check + +// CloneAndDisable returns a copy of this Checks slice with all checks matching prefix disabled. +func (c Checks) CloneAndDisable(prefixes ...string) Checks { + out := Checks{} +skip: + for _, check := range c { + id := check.ID() + for _, prefix := range prefixes { + if prefix == id || strings.HasPrefix(id, prefix+".") { + continue skip + } + } + out = append(out, check) + } + return out +} + +// Has returns true if the Checks slice contains any checks matching prefix. +func (c Checks) Has(prefix string) bool { + for _, check := range c { + id := check.ID() + if prefix == id || strings.HasPrefix(id, prefix+".") { + return true + } + } + return false +} + +// Check implementations are used by the linter to check AST nodes. +type Check interface { + // ID of the Check. Must be unique across all checks. + // + // IDs may be hierarchical, separated by a period. eg. "enum", "enum.values" + ID() string + // Checker returns the checking function. + // + // The checking function has the signature "func(...) Messages", where "..." is a sequence of + // Thrift AST types that are matched against the current node's ancestors as the linter walks + // the AST of each file. "..." may also be "interface{}" in which case the checker function + // will be called for each node in the AST. + // + // For example, the function: + // + // func (s *parser.Struct, f *parser.Field) (messages Messages) + // + // Will match all each struct field, but not union fields. + Checker() interface{} +} + +// MakeCheck creates a stateless Check type from an ID and a checker function. +func MakeCheck(id string, checker interface{}) Check { + return &statelessCheck{ + id: id, + checker: checker, + } +} + +type statelessCheck struct { + id string + checker interface{} +} + +func (s *statelessCheck) ID() string { + return s.id +} + +func (s *statelessCheck) Checker() interface{} { + return s.checker +} diff --git a/api_test.go b/api_test.go new file mode 100644 index 0000000..85b9008 --- /dev/null +++ b/api_test.go @@ -0,0 +1,24 @@ +package thriftlint + +import ( + "github.com/stretchr/testify/require" + + "testing" +) + +func TestChecks(t *testing.T) { + checks := Checks{ + MakeCheck("alpha", nil), + MakeCheck("alpha.beta", nil), + MakeCheck("beta.gamma", nil), + MakeCheck("beta.zeta", nil), + } + + actual := checks.CloneAndDisable("alpha") + expected := Checks{checks[2], checks[3]} + require.Equal(t, expected, actual) + + actual = checks.CloneAndDisable("alpha.beta") + expected = Checks{checks[0], checks[2], checks[3]} + require.Equal(t, expected, actual) +} diff --git a/checks/annotations.go b/checks/annotations.go new file mode 100644 index 0000000..8c9d0b4 --- /dev/null +++ b/checks/annotations.go @@ -0,0 +1,122 @@ +package checks + +import ( + "reflect" + "regexp" + "strings" + + "github.com/alecthomas/go-thrift/parser" + + "github.com/UrbanCompass/thriftlint" +) + +// Accepts urls in the form /(|:)+[/] +const urlRegex = "(?:/(?:(?:[a-z0-9_]+)|(:[a-zA-Z_][a-zA-Z0-9_]+)))+/?" + +// var ( +// annotationRegistry = map[reflect.Type]map[string]string{ +// thriftlint.ServiceType: { +// "api.url": urlRegex, +// "api.proxy": urlRegex, +// "api.test": "^$", +// }, +// thriftlint.MethodType: { +// "api.url": urlRegex, +// "api.method": "GET|POST|DELETE|PUT", +// "api.roles": ".*", +// }, +// // TODO(cameron): Generate the pattern here from api.InjectorMap once it's submitted. +// thriftlint.FieldType: { +// "api.inject": "^$", +// "api.path": "^$", +// }, +// } +// ) + +type AnnotationPattern struct { + //AST nodes this annotation pattern should apply to. + Nodes []reflect.Type + Annotation string + Regex string +} + +type annotationsCheck struct { + patterns map[reflect.Type]map[string]string + checks thriftlint.Checks +} + +// CheckAnnotations validates Thrift annotations against regular expressions. +// +// All supported annotations must be represented. +// +// NOTE: This should be the last check added in order to correctly validate the allowed values +// for "nolint". +func CheckAnnotations(patterns []*AnnotationPattern, checks thriftlint.Checks) thriftlint.Check { + patternsLUT := map[reflect.Type]map[string]string{} + for _, pattern := range patterns { + for _, node := range pattern.Nodes { + mapping, ok := patternsLUT[node] + if !ok { + mapping = map[string]string{} + patternsLUT[node] = mapping + } + mapping[pattern.Annotation] = pattern.Regex + } + } + return &annotationsCheck{ + patterns: patternsLUT, + checks: checks, + } +} + +func (c *annotationsCheck) ID() string { + return "annotations" +} + +func (c *annotationsCheck) Checker() interface{} { + return c.checker +} + +// annotationsCheck verifies that annotations match basic regular expressions. +// +// It does not do any semantic checking. +func (c *annotationsCheck) checker(self interface{}) (messages thriftlint.Messages) { + v := reflect.Indirect(reflect.ValueOf(self)) + var annotations []*parser.Annotation + if annotationsField := v.FieldByName("Annotations"); annotationsField.IsValid() { + annotations = annotationsField.Interface().([]*parser.Annotation) + } + // Validate type-specific annotations. + if checks, ok := c.patterns[v.Type()]; ok { + for _, annotation := range annotations { + if pattern, ok := checks[annotation.Name]; ok { + re := regexp.MustCompile("^(?:" + pattern + ")$") + if !re.MatchString(annotation.Value) { + messages.Warning(annotation, "invalid value %q for annotation %q (should match %q)", + annotation.Value, annotation.Name, pattern) + } + } else if annotation.Name != "nolint" { + messages.Warning(annotation, "unsupported annotation %q", annotation.Name) + } + } + } else { + for _, annotation := range annotations { + if annotation.Name != "nolint" { + messages.Warning(annotation, "unsupported annotation %q", annotation.Name) + } + } + } + + // Validate `nolint` annotation contains only valid checks to be disabled. + for _, annotation := range annotations { + if annotation.Name == "nolint" && annotation.Value != "" { + lints := strings.Split(annotation.Value, ",") + for _, l := range lints { + if !c.checks.Has(l) { + messages.Warning(annotation, "%q is not a known linter check", l) + } + } + } + } + return +} diff --git a/checks/defaults.go b/checks/defaults.go new file mode 100644 index 0000000..f81568b --- /dev/null +++ b/checks/defaults.go @@ -0,0 +1,17 @@ +package checks + +import ( + "github.com/UrbanCompass/thriftlint" + + "github.com/alecthomas/go-thrift/parser" +) + +// CheckDefaultValues checks that default values are not provided. +func CheckDefaultValues() thriftlint.Check { + return thriftlint.MakeCheck("defaults", func(field *parser.Field) (messages thriftlint.Messages) { + if field.Default != nil { + messages.Warning(field, "default values are not allowed") + } + return + }) +} diff --git a/checks/doc.go b/checks/doc.go new file mode 100644 index 0000000..a17daa5 --- /dev/null +++ b/checks/doc.go @@ -0,0 +1,2 @@ +// Package checks contains default checks included with the Thrift linter. +package checks diff --git a/checks/enums.go b/checks/enums.go new file mode 100644 index 0000000..50a262c --- /dev/null +++ b/checks/enums.go @@ -0,0 +1,28 @@ +package checks + +import ( + "sort" + + "github.com/UrbanCompass/thriftlint" + + "github.com/alecthomas/go-thrift/parser" +) + +// CheckEnumSequence checks that enums start with 0 and increment sequentially. +func CheckEnumSequence() thriftlint.Check { + return thriftlint.MakeCheck("enum", func(e *parser.Enum) (messages thriftlint.Messages) { + values := []int{} + for _, v := range e.Values { + values = append(values, v.Value) + } + sort.Sort(sort.IntSlice(values)) + for i := 0; i < len(values); i++ { + if values[i] != i { + messages.Warning(e, + "enum values for %s do not start at 0 and increase monotonically", e.Name) + break + } + } + return + }) +} diff --git a/checks/fields.go b/checks/fields.go new file mode 100644 index 0000000..201d5f2 --- /dev/null +++ b/checks/fields.go @@ -0,0 +1,31 @@ +package checks + +import ( + "sort" + + "github.com/alecthomas/go-thrift/parser" + + "github.com/UrbanCompass/thriftlint" +) + +type sortedFields []*parser.Field + +func (s sortedFields) Len() int { return len(s) } +func (s sortedFields) Swap(i, j int) { s[i], s[j] = s[j], s[i] } +func (s sortedFields) Less(i, j int) bool { return s[i].ID < s[j].ID } + +// CheckStructFieldOrder ensures that struct field IDs are present in-order in the file. +func CheckStructFieldOrder() thriftlint.Check { + return thriftlint.MakeCheck("field.order", func(s *parser.Struct) (messages thriftlint.Messages) { + fields := sortedFields(s.Fields) + sort.Sort(fields) + for i := 0; i < len(fields)-1; i++ { + a := fields[i] + b := fields[i+1] + if a.Pos.Line > b.Pos.Line { + messages.Warning(fields[i], "field %d and %d are out of order", a.ID, b.ID) + } + } + return + }) +} diff --git a/checks/indentation.go b/checks/indentation.go new file mode 100644 index 0000000..2d5c0de --- /dev/null +++ b/checks/indentation.go @@ -0,0 +1,38 @@ +package checks + +import ( + "reflect" + + "github.com/UrbanCompass/thriftlint" +) + +var ( + // Map of (parent, self) AST node types to the expected indentation for that type in that + // context. + expectedIndentation = map[string]int{ + indentationContext(thriftlint.ThriftType, thriftlint.ServiceType): 1, + indentationContext(thriftlint.ServiceType, thriftlint.MethodType): 3, + indentationContext(thriftlint.ThriftType, thriftlint.EnumType): 1, + indentationContext(thriftlint.EnumType, thriftlint.EnumValueType): 3, + indentationContext(thriftlint.ThriftType, thriftlint.StructType): 8, + indentationContext(thriftlint.StructType, thriftlint.FieldType): 3, + indentationContext(thriftlint.ThriftType, thriftlint.TypedefType): 1, + indentationContext(thriftlint.ThriftType, thriftlint.ConstantType): 1, + } +) + +func indentationContext(parent, self reflect.Type) string { + return parent.Name() + ":" + self.Name() +} + +// CheckIndentation checks indentation is a multiple of 2. +func CheckIndentation() thriftlint.Check { + return thriftlint.MakeCheck("indentation", func(parent, self interface{}) (messages thriftlint.Messages) { + context := indentationContext(reflect.TypeOf(parent), reflect.TypeOf(self)) + pos := thriftlint.Pos(self) + if expected, ok := expectedIndentation[context]; ok && expected != pos.Col { + messages.Warning(self, "should be indented to column %d not %d", expected, pos.Col) + } + return + }) +} diff --git a/checks/maps.go b/checks/maps.go new file mode 100644 index 0000000..23813f7 --- /dev/null +++ b/checks/maps.go @@ -0,0 +1,30 @@ +package checks + +import ( + "github.com/UrbanCompass/thriftlint" + "github.com/alecthomas/go-thrift/parser" +) + +// CheckMapKeys verifies that map keys are valid types. +func CheckMapKeys() thriftlint.Check { + return thriftlint.MakeCheck("map", checkMapKeys) +} + +func checkMapKeys(file *parser.Thrift, t *parser.Type) (messages thriftlint.Messages) { + if t.Name == "map" { + kn := t.KeyType.Name + if kn != "string" && kn != "i16" && kn != "i32" && kn != "i64" && kn != "double" { + // Not an integral type, check if it's an enum and allow it if so. + resolved := thriftlint.Resolve(kn, file) + isEnum := false + if resolved != nil { + _, isEnum = resolved.(*parser.Enum) + } + if !isEnum { + messages.Error(t, "map keys must be string, enum, integer or double, not %q", kn) + } + } + return checkMapKeys(file, t.ValueType) + } + return +} diff --git a/checks/naming.go b/checks/naming.go new file mode 100644 index 0000000..477f598 --- /dev/null +++ b/checks/naming.go @@ -0,0 +1,72 @@ +package checks + +import ( + "reflect" + "regexp" + "strings" + + "github.com/UrbanCompass/thriftlint" +) + +var ( + upperCamelCaseRegex = `^[_A-Z][a-z]*([A-Z][0-9a-z]*)*$` + lowerCamelCaseRegex = `^[_a-z]+([A-Z0-9a-z]*)*$` + upperSnakeCaseRegex = `^[A-Z_]+([A-Z0-9]+_?)*$` +) + +var ( + // CheckNamesDefaults is a map of Thrift AST node type to a regular expression for + // validating names of that type. + CheckNamesDefaults = map[reflect.Type]string{ + thriftlint.ServiceType: upperCamelCaseRegex, + thriftlint.EnumType: upperCamelCaseRegex, + thriftlint.StructType: upperCamelCaseRegex, + thriftlint.EnumValueType: upperSnakeCaseRegex, + thriftlint.FieldType: lowerCamelCaseRegex, + thriftlint.MethodType: lowerCamelCaseRegex, + thriftlint.ConstantType: upperSnakeCaseRegex, + } + + // CheckNamesDefaultBlacklist is names that should never be used for symbols. + CheckNamesDefaultBlacklist = map[string]bool{ + "class": true, + "int": true, + } +) + +// CheckNames checks Thrift symbols comply with a set of regular expressions. +// +// If mathces or blacklist are nil, global defaults will be used. +func CheckNames(matches map[reflect.Type]string, blacklist map[string]bool) thriftlint.Check { + if matches == nil { + matches = CheckNamesDefaults + } + if blacklist == nil { + blacklist = CheckNamesDefaultBlacklist + } + regexes := map[reflect.Type]*regexp.Regexp{} + for t, p := range matches { + regexes[t] = regexp.MustCompile(p) + } + return thriftlint.MakeCheck("naming", func(v interface{}) (messages thriftlint.Messages) { + rv := reflect.Indirect(reflect.ValueOf(v)) + nameField := rv.FieldByName("Name") + if !nameField.IsValid() { + return nil + } + name := nameField.Interface().(string) + // Special-case DEPRECATED_ fields. + checker, ok := regexes[rv.Type()] + if !ok || strings.HasPrefix(name, "DEPRECATED_") { + return nil + } + if blacklist[name] { + messages.Warning(v, "%q is a disallowed name", name) + } + if ok := checker.MatchString(name); !ok { + messages.Warning(v, "name of %s %q should match %q", strings.ToLower(rv.Type().Name()), + name, checker.String()) + } + return + }) +} diff --git a/checks/optional.go b/checks/optional.go new file mode 100644 index 0000000..fc48760 --- /dev/null +++ b/checks/optional.go @@ -0,0 +1,18 @@ +package checks + +import ( + "github.com/UrbanCompass/thriftlint" + + "github.com/alecthomas/go-thrift/parser" +) + +// CheckOptional ensures that all Thrift fields are optional, as is generally accepted best +// practice for Thrift. +func CheckOptional() thriftlint.Check { + return thriftlint.MakeCheck("optional", func(s *parser.Struct, f *parser.Field) (messages thriftlint.Messages) { + if f.Type.Name != "list" && f.Type.Name != "set" && f.Type.Name != "map" && !f.Optional { + messages.Warning(f, "%s must be optional", f.Name) + } + return + }) +} diff --git a/checks/types.go b/checks/types.go new file mode 100644 index 0000000..95b4ba1 --- /dev/null +++ b/checks/types.go @@ -0,0 +1,19 @@ +package checks + +import ( + "github.com/alecthomas/go-thrift/parser" + + "github.com/UrbanCompass/thriftlint" +) + +// CheckTypeReferences checks that types referenced in Thrift files are actually imported +// and exist. +func CheckTypeReferences() thriftlint.Check { + return thriftlint.MakeCheck("types", func(file *parser.Thrift, t *parser.Type) (messages thriftlint.Messages) { + if !thriftlint.BuiltinThriftTypes[t.Name] && !thriftlint.BuiltinThriftCollections[t.Name] && + thriftlint.Resolve(t.Name, file) == nil { + messages.Error(t, "unknown type %q", t.Name) + } + return + }) +} diff --git a/cmd/thrift-lint/main.go b/cmd/thrift-lint/main.go new file mode 100644 index 0000000..92f8be6 --- /dev/null +++ b/cmd/thrift-lint/main.go @@ -0,0 +1,71 @@ +package main + +import ( + "fmt" + "log" + "os" + + "gopkg.in/alecthomas/kingpin.v3-unstable" + + "github.com/UrbanCompass/thriftlint" + "github.com/UrbanCompass/thriftlint/checks" +) + +var ( + includeDirsFlag = kingpin.Flag("include", "Include directories to search.").Short('I').PlaceHolder("DIR").ExistingDirs() + debugFlag = kingpin.Flag("debug", "Enable debug logging.").Bool() + disableFlag = kingpin.Flag("disable", "Linters to disable.").PlaceHolder("LINTER").Strings() + listFlag = kingpin.Flag("list", "List linter checks.").Bool() + errorFlag = kingpin.Flag("errors", "Only show errors.").Bool() + sourcesArgs = kingpin.Arg("sources", "Thrift sources to lint.").Required().ExistingFiles() +) + +func main() { + kingpin.CommandLine.Help = `A linter for Thrift. + +For details, please refer to https://github.com/UrbanCompass/thriftlint +` + kingpin.Parse() + checkers := thriftlint.Checks{ + checks.CheckIndentation(), + checks.CheckNames(nil, nil), + checks.CheckOptional(), + checks.CheckDefaultValues(), + checks.CheckEnumSequence(), + checks.CheckMapKeys(), + checks.CheckTypeReferences(), + checks.CheckStructFieldOrder(), + } + checkers = append(checkers, checks.CheckAnnotations(nil, checkers)) + + if *listFlag { + for _, linter := range checkers { + fmt.Printf("%s\n", linter.ID()) + } + return + } + + options := []thriftlint.Option{ + thriftlint.WithIncludeDirs(*includeDirsFlag...), + thriftlint.Disable(*disableFlag...), + } + if *debugFlag { + logger := log.New(os.Stdout, "debug: ", 0) + options = append(options, thriftlint.WithLogger(logger)) + } + linter, err := thriftlint.New(checkers, options...) + kingpin.FatalIfError(err, "") + messages, err := linter.Lint(*sourcesArgs) + kingpin.FatalIfError(err, "") + status := 0 + for _, msg := range messages { + if *errorFlag && msg.Severity != thriftlint.Error { + continue + } + pos := thriftlint.Pos(msg.Object) + fmt.Fprintf(os.Stderr, "%s:%d:%d:%s: %s (%s)\n", msg.File.Filename, pos.Line, pos.Col, + msg.Severity, msg.Message, msg.Checker) + status |= 1 << uint(msg.Severity) + } + os.Exit(status) +} diff --git a/context.go b/context.go new file mode 100644 index 0000000..48bd55b --- /dev/null +++ b/context.go @@ -0,0 +1,46 @@ +package thriftlint + +import ( + "strings" + + "github.com/alecthomas/go-thrift/parser" +) + +// Resolve a symbol within a file to its type. +func Resolve(symbol string, file *parser.Thrift) interface{} { + parts := strings.SplitN(symbol, ".", 2) + name := symbol + target := file + if len(parts) == 2 { + target = file.Imports[parts[0]] + if target == nil { + return nil + } + name = parts[1] + if target == nil { + return nil + } + } + if t, ok := target.Constants[name]; ok { + return t + } + if t, ok := target.Enums[name]; ok { + return t + } + if t, ok := target.Exceptions[name]; ok { + return t + } + if t, ok := target.Services[name]; ok { + return t + } + if t, ok := target.Structs[name]; ok { + return t + } + if t, ok := target.Typedefs[name]; ok { + return t + } + if t, ok := target.Unions[name]; ok { + return t + } + return nil +} diff --git a/context_test.go b/context_test.go new file mode 100644 index 0000000..e6d8dd1 --- /dev/null +++ b/context_test.go @@ -0,0 +1,67 @@ +package thriftlint + +import ( + "github.com/alecthomas/go-thrift/parser" + "github.com/stretchr/testify/require" + + "testing" +) + +func TestResolve(t *testing.T) { + parsed, err := parser.Parse("test.thrift", []byte(` +struct Struct {}; +const string CONST = ""; +enum Enum { CASE = 1; }; +exception Exception {}; +service Service {}; +typedef Service Typedef; +union Union {}; +`)) + require.NoError(t, err) + ast := parsed.(*parser.Thrift) + ast.Imports = map[string]*parser.Thrift{ + "pkg": ast, + } + + actual := Resolve("Struct", ast) + require.NotNil(t, actual) + require.Equal(t, ast.Structs["Struct"], actual) + actual = Resolve("pkg.Struct", ast) + require.NotNil(t, actual) + require.Equal(t, ast.Structs["Struct"], actual) + + actual = Resolve("CONST", ast) + require.NotNil(t, actual) + require.Equal(t, ast.Constants["CONST"], actual) + actual = Resolve("pkg.CONST", ast) + require.NotNil(t, actual) + require.Equal(t, ast.Constants["CONST"], actual) + + actual = Resolve("Enum", ast) + require.NotNil(t, actual) + require.Equal(t, ast.Enums["Enum"], actual) + actual = Resolve("pkg.Enum", ast) + require.NotNil(t, actual) + require.Equal(t, ast.Enums["Enum"], actual) + + actual = Resolve("Service", ast) + require.NotNil(t, actual) + require.Equal(t, ast.Services["Service"], actual) + actual = Resolve("pkg.Service", ast) + require.NotNil(t, actual) + require.Equal(t, ast.Services["Service"], actual) + + actual = Resolve("Typedef", ast) + require.NotNil(t, actual) + require.Equal(t, ast.Typedefs["Typedef"], actual) + actual = Resolve("pkg.Typedef", ast) + require.NotNil(t, actual) + require.Equal(t, ast.Typedefs["Typedef"], actual) + + actual = Resolve("Union", ast) + require.NotNil(t, actual) + require.Equal(t, ast.Unions["Union"], actual) + actual = Resolve("pkg.Union", ast) + require.NotNil(t, actual) + require.Equal(t, ast.Unions["Union"], actual) +} diff --git a/doc.go b/doc.go new file mode 100644 index 0000000..b8ad2ee --- /dev/null +++ b/doc.go @@ -0,0 +1,7 @@ +// Package thriftlint is an extensible Linter for Thrift files, written in Go. +// +// New() takes a set of checks and options and creates a linter. The linter checks can then +// be applied to a set of files with .Lint(files...). +// +// See the README for details. +package thriftlint diff --git a/linter.go b/linter.go new file mode 100644 index 0000000..46da973 --- /dev/null +++ b/linter.go @@ -0,0 +1,220 @@ +// Package linter lints Thrift files. +// +// Actual implementations of linter checks are in the subpackage "checks". +package thriftlint + +import ( + "io/ioutil" + "log" + "reflect" + "strings" + + "github.com/alecthomas/go-thrift/parser" + // Imported to register checkers. +) + +type logger interface { + Printf(format string, args ...interface{}) +} + +type Linter struct { + checkers Checks + includeDirs []string + log logger +} + +type Option func(*Linter) + +// WithIncludeDirs is an Option that sets the directories to use for searching when parsing Thrift includes. +func WithIncludeDirs(dirs ...string) Option { + return func(l *Linter) { l.includeDirs = dirs } +} + +// WithLogger is an Option that sets the logger object used by the linter. +func WithLogger(logger logger) Option { + return func(l *Linter) { l.log = logger } +} + +// Disable is an Option that disables the given checks. +func Disable(checks ...string) Option { + return func(l *Linter) { + l.checkers = l.checkers.CloneAndDisable(checks...) + } +} + +// New creates a new Linter. +func New(checks []Check, options ...Option) (*Linter, error) { + ids := []string{} + for _, check := range checks { + ids = append(ids, check.ID()) + } + l := &Linter{ + checkers: Checks(checks), + log: log.New(ioutil.Discard, "", 0), + } + for _, option := range options { + option(l) + } + l.log.Printf("Linting with: %s", strings.Join(ids, ", ")) + return l, nil +} + +// Lint the given files. +func (l *Linter) Lint(sources []string) (Messages, error) { + l.log.Printf("Parsing %d files", len(sources)) + files, err := Parse(l.includeDirs, sources) + if err != nil { + return nil, err + } + messages := Messages{} + for _, file := range files { + l.log.Printf("Linting %s", file.Filename) + v := reflect.ValueOf(file) + enabledChecks := l.checkers.CloneAndDisable() + // Seed the "ancestors" with imports. + ancestors := []interface{}{file.Imports} + messages = append(messages, l.walk(file, ancestors, v, enabledChecks)...) + } + return messages, nil +} + +// Apply checks to all Thrift objects in the file. +// +// parent is the last parent struct encountered. +// enabledChecks are updated recursively as (nolint[="check check,..."]) annotations are +// found in the AST. +func (l *Linter) walk(file *parser.Thrift, ancestors []interface{}, v reflect.Value, + enabledChecks Checks) (messages Messages) { + originalNode := v + v = reflect.Indirect(v) + switch v.Kind() { + case reflect.Struct: + // Update enabledChecks. + var annotations []*parser.Annotation + if annotationsField := v.FieldByName("Annotations"); annotationsField.IsValid() { + annotations = annotationsField.Interface().([]*parser.Annotation) + for _, a := range annotations { + if a.Name == "nolint" { + // Skip linting altogether if all checks are disabled. + if a.Value == "" { + return + } + enabledChecks = enabledChecks.CloneAndDisable(strings.Fields(a.Value)...) + } + } + } + + ancestors = append(ancestors, originalNode.Interface()) + for _, checker := range enabledChecks { + id := checker.ID() + for _, msg := range callChecker(checker.Checker(), ancestors) { + msg.File = file + msg.Checker = id + messages = append(messages, msg) + } + } + + for i := 0; i < v.NumField(); i++ { + ft := v.Type().Field(i) + if ft.Name == "Pos" || (ft.Name == "Imports" && v.Type() == reflect.TypeOf(parser.Thrift{})) { + continue + } + messages = append(messages, l.walk(file, ancestors, v.Field(i), enabledChecks)...) + } + + case reflect.Slice: + for i := 0; i < v.Len(); i++ { + messages = append(messages, l.walk(file, ancestors, v.Index(i), enabledChecks)...) + } + + case reflect.Map: + for _, key := range v.MapKeys() { + messages = append(messages, l.walk(file, ancestors, v.MapIndex(key), enabledChecks)...) + } + } + return +} + +// Apparently it's non-trivial to get the type of the empty interface... +var emptyInterfaceValue interface{} +var emptyInterfaceType = reflect.TypeOf(&emptyInterfaceValue).Elem() + +// Call a checker function if its arguments end with the last element in ancestors, and all other +// arguments are present in ancestors, in order. +// +// For example, given ancestors = {*parser.Thrift, *parser.Struct, *parser.Field} +// the following functions would match: +// +// f(*parser.Thrift, *parser.Struct, *parser.Field) +// f(*parser.Struct, *parser.Field) +// f(*parser.Thrift, *parser.Field) +// f(*parser.Field) +// +// But these would not: +// +// f(*parser.Thrift) +// f(*parser.Struct) +// f(*parser.Field, *parser.Struct) +// +func callChecker(checker interface{}, ancestors []interface{}) Messages { + l := reflect.TypeOf(checker) + if l.Kind() != reflect.Func { + panic("checker must be a function but is " + l.String()) + } + if l.NumOut() != 1 || l.Out(0) != reflect.TypeOf(Messages{}) { + panic("checkers must return exactly Messages") + } + + args := []reflect.Value{} + switch { + // func(self interface{}) + case l.NumIn() == 1 && l.In(0) == emptyInterfaceType: + args = append(args, reflect.ValueOf(ancestors[len(ancestors)-1])) + + // func(parent, self interface{}) + case l.NumIn() == 2 && l.In(0) == emptyInterfaceType && l.In(1) == emptyInterfaceType: + if len(ancestors) < 2 { + return nil + } + args = append(args, + reflect.ValueOf(ancestors[len(ancestors)-2]), + reflect.ValueOf(ancestors[len(ancestors)-1]), + ) + + default: + // Ensure last argument matches last ancestor. + if reflect.TypeOf(ancestors[len(ancestors)-1]) != l.In(l.NumIn()-1) { + return nil + } + + ancestorsValues := []reflect.Value{} + for _, a := range ancestors { + ancestorsValues = append(ancestorsValues, reflect.ValueOf(a)) + } + + ancestorIndex := len(ancestorsValues) - 1 + for parameterIndex := l.NumIn() - 1; ancestorIndex >= 0 && parameterIndex >= 0; parameterIndex-- { + for ancestorIndex >= 0 { + arg := ancestorsValues[ancestorIndex] + if arg.Type().ConvertibleTo(l.In(parameterIndex)) { + args = append(args, arg) + break + } + ancestorIndex-- + } + } + + // Arguments did not match. + if len(args) != l.NumIn() { + return nil + } + + // Reverse args to correct order. + for i, j := 0, len(args)-1; i < j; i, j = i+1, j-1 { + args[i], args[j] = args[j], args[i] + } + + } + out := reflect.ValueOf(checker).Call(args) + return out[0].Interface().(Messages) +} diff --git a/linter_test.go b/linter_test.go new file mode 100644 index 0000000..29729a6 --- /dev/null +++ b/linter_test.go @@ -0,0 +1,43 @@ +package thriftlint + +import ( + "testing" + + "github.com/alecthomas/go-thrift/parser" + "github.com/stretchr/testify/require" +) + +func TestCallCheckerValidation(t *testing.T) { + failf := func(*parser.Thrift) {} + require.Panics(t, func() { callChecker(failf, []interface{}{}) }) + okf := func(*parser.Thrift) Messages { return nil } + require.NotPanics(t, func() { callChecker(okf, []interface{}{&parser.Thrift{}}) }) +} + +func TestCallChecker(t *testing.T) { + okfuncs := []interface{}{ + func(*parser.Thrift, *parser.Struct, *parser.Field) Messages { + return Messages{} + }, + func(*parser.Struct, *parser.Field) Messages { return Messages{} }, + func(*parser.Thrift, *parser.Field) Messages { return Messages{} }, + func(*parser.Field) Messages { return Messages{} }, + func(self interface{}) Messages { return Messages{} }, + func(parent, self interface{}) Messages { return Messages{} }, + } + ancestors := []interface{}{&parser.Thrift{}, &parser.Struct{}, &parser.Field{}} + for _, okf := range okfuncs { + out := callChecker(okf, ancestors) + require.NotNil(t, out) + } + + badfuncs := []interface{}{ + func(*parser.Thrift) Messages { return Messages{} }, + func(*parser.Struct) Messages { return Messages{} }, + func(*parser.Field, *parser.Struct) Messages { return Messages{} }, + } + for _, badf := range badfuncs { + out := callChecker(badf, ancestors) + require.Nil(t, out) + } +} diff --git a/parse.go b/parse.go new file mode 100644 index 0000000..c266d11 --- /dev/null +++ b/parse.go @@ -0,0 +1,73 @@ +package thriftlint + +import ( + "io" + "os" + "path/filepath" + + "github.com/alecthomas/go-thrift/parser" +) + +// Parse a set of .thrift source files into their corresponding ASTs. +func Parse(includeDirs []string, sources []string) (map[string]*parser.Thrift, error) { + p := parser.New() + p.Filesystem = &includeFilesystem{IncludeDirs: includeDirs} + + var files map[string]*parser.Thrift + for _, path := range sources { + path, err := filepath.Abs(path) + if err != nil { + return nil, err + } + files, _, err = p.ParseFile(path) + if err != nil { + return nil, err + } + } + for _, file := range files { + file.Imports = map[string]*parser.Thrift{} + for symbol, path := range file.Includes { + file.Imports[symbol] = files[path] + } + } + return files, nil +} + +// A go-thrift/parser.Filesystem implementation that searches include dirs when attempting to open +// sources. +type includeFilesystem struct { + IncludeDirs []string +} + +func (i *includeFilesystem) Open(filename string) (io.ReadCloser, error) { + if filepath.IsAbs(filename) { + return os.Open(filename) + } + for _, d := range i.IncludeDirs { + path := filepath.Join(d, filename) + r, err := os.Open(path) + if os.IsNotExist(err) { + continue + } else if err != nil { + return nil, err + } + return r, nil + } + return nil, os.ErrNotExist +} + +func (i *includeFilesystem) Abs(dir, path string) (string, error) { + if filepath.IsAbs(path) { + return path, nil + } + for _, d := range i.IncludeDirs { + p, err := filepath.Abs(filepath.Join(d, path)) + if err != nil { + continue + } + if _, err := os.Stat(p); err == nil { + return filepath.Abs(p) + } + } + return filepath.Abs(filepath.Join(dir, path)) +} diff --git a/symbol.go b/symbol.go new file mode 100644 index 0000000..7f4bb24 --- /dev/null +++ b/symbol.go @@ -0,0 +1,269 @@ +package thriftlint + +import ( + "bytes" + "fmt" + "go/doc" + "reflect" + "strings" + "unicode" +) + +var ( + // BuiltinThriftTypes is a map of the basic builtin Thrift types. Useful in templates. + BuiltinThriftTypes = map[string]bool{ + "bool": true, + "byte": true, + "i16": true, + "i32": true, + "i64": true, + "double": true, + "string": true, + } + + // BuiltinThriftCollections is the set of builtin collection types in Thrift. + BuiltinThriftCollections = map[string]bool{ + "map": true, + "list": true, + "set": true, + "binary": true, + } +) + +// Scanner over a sequence of runes. +type scanner struct { + runes []rune + cursor int +} + +func (s *scanner) peek() rune { + if s.cursor >= len(s.runes) { + return -1 + } + return s.runes[s.cursor] +} + +func (s *scanner) next() rune { + r := s.peek() + if r != -1 { + s.cursor++ + } + return r +} + +func (s *scanner) reverse() { + s.cursor-- +} + +func consumeLower(scan *scanner) string { + out := "" + for unicode.IsLower(scan.peek()) || unicode.IsNumber(scan.peek()) { + out += string(scan.next()) + } + return out +} + +// ([A-Z]+)(?:[A-Z][a-z]|$) +func consumeMostUpper(scan *scanner) string { + out := "" + for unicode.IsUpper(scan.peek()) || unicode.IsNumber(scan.peek()) { + r := scan.next() + if unicode.IsLower(scan.peek()) && !commonInitialisms[out+string(r)] { + scan.reverse() + break + } + out += string(r) + } + return out +} + +func title(s string) string { + return strings.ToUpper(s[0:1]) + strings.ToLower(s[1:]) +} + +// From https://github.com/golang/lint/blob/master/lint.go +var commonInitialisms = map[string]bool{ + "API": true, + "ASCII": true, + "CPU": true, + "CSS": true, + "DB": true, + "DNS": true, + "EOF": true, + "GUID": true, + "HTML": true, + "HTTP": true, + "HTTPS": true, + "ID": true, + "IP": true, + "JSON": true, + "LHS": true, + "MD5": true, + "MLS": true, + "OK": true, + "QPS": true, + "RAM": true, + "RHS": true, + "RPC": true, + "SHA": true, + "SLA": true, + "SMTP": true, + "SQL": true, + "SSH": true, + "TCP": true, + "TLS": true, + "TTL": true, + "UDP": true, + "UI": true, + "UID": true, + "URI": true, + "URL": true, + "UTC": true, + "UTF8": true, + "UUID": true, + "VM": true, + "XML": true, + "XSRF": true, + "XSS": true, +} + +func Comment(v interface{}) []string { + comment := reflect.Indirect(reflect.ValueOf(v)).FieldByName("Comment").Interface().(string) + if comment == "" { + return nil + } + w := bytes.NewBuffer(nil) + doc.ToText(w, comment, "", "", 80) + return strings.Split(strings.TrimSpace(w.String()), "\n") +} + +func IsInitialism(s string) bool { + return commonInitialisms[strings.ToUpper(s)] +} + +// UpperCamelCase converts a symbol to CamelCase +func UpperCamelCase(s string) string { + parts := []string{} + for _, part := range SplitSymbol(s) { + if part == "" { + parts = append(parts, "_") + continue + } + if part == "s" && len(parts) > 0 { + parts[len(parts)-1] += part + } else { + if commonInitialisms[strings.ToUpper(part)] { + part = strings.ToUpper(part) + } else { + part = title(part) + } + parts = append(parts, part) + } + } + return strings.Join(parts, "") +} + +// LowerCamelCase converts a symbol to lowerCamelCase +func LowerCamelCase(s string) string { + first := true + parts := []string{} + for _, part := range SplitSymbol(s) { + if part == "" { + parts = append(parts, "_") + continue + } + if first { + parts = append(parts, strings.ToLower(part)) + first = false + } else { + // Merge trailing s + if part == "s" && len(parts) > 0 { + parts[len(parts)-1] += part + } else { + if commonInitialisms[strings.ToUpper(part)] { + part = strings.ToUpper(part) + } else { + part = title(part) + } + parts = append(parts, part) + } + } + } + return strings.Join(parts, "") +} + +// LowerSnakeCase converts a symbol to snake_case +func LowerSnakeCase(s string) string { + parts := []string{} + for _, part := range SplitSymbol(s) { + if part == "" { + parts = append(parts, "_") + continue + } + parts = append(parts, strings.ToLower(part)) + } + return strings.Join(parts, "_") +} + +// UpperSnakeCase converts a symbol to UPPER_SNAKE_CASE +func UpperSnakeCase(s string) string { + parts := []string{} + for _, part := range SplitSymbol(s) { + if part == "" { + parts = append(parts, "_") + continue + } + parts = append(parts, strings.ToUpper(part)) + } + return strings.Join(parts, "_") +} + +// SplitSymbol splits an arbitrary symbol into parts. It accepts symbols in snake case and camel +// case, and correctly supports all-caps substrings. +// +// eg. "some_snake_case_symbol" would become ["some", "snake", "case", "symbol"] +// and "someCamelCaseSymbol" would become ["some", "Camel", "Case", "Symbol"] +func SplitSymbol(s string) []string { + // This is painful. See TestSplitSymbol for examples of what this does. + out := []string{} + scan := &scanner{runes: []rune(s)} + for scan.peek() != -1 { + part := "" + r := scan.peek() + switch { + case unicode.IsLower(r): + part = consumeLower(scan) + case unicode.IsUpper(r): + scan.next() + // [A-Z][a-z]+ + if unicode.IsLower(scan.peek()) { + part += string(r) + part += consumeLower(scan) + } else { + scan.reverse() + part += consumeMostUpper(scan) + } + case unicode.IsNumber(r): + for unicode.IsNumber(scan.peek()) { + part += string(scan.next()) + } + case r == '_': + scan.next() + if len(out) == 0 { + break + } + continue + default: + panic(fmt.Sprintf("unsupported character %q in %q", r, s)) + } + out = append(out, part) + } + return out +} + +// Extract the suffix from a . separated string (ie. namespace). +// Useful for getting the package reference from a files namespace. +func DotSuffix(pkg string) string { + parts := strings.Split(pkg, ".") + return parts[len(parts)-1] +} diff --git a/symbol_test.go b/symbol_test.go new file mode 100644 index 0000000..9a9e2ca --- /dev/null +++ b/symbol_test.go @@ -0,0 +1,67 @@ +package thriftlint + +import ( + "strings" + + "github.com/alecthomas/go-thrift/parser" + "github.com/stretchr/testify/require" + + "testing" +) + +func TestSplitSymbol(t *testing.T) { + actual := SplitSymbol("someSnakeCaseAPI") + require.Equal(t, []string{"some", "Snake", "Case", "API"}, actual) + actual = SplitSymbol("someSnakeCase") + require.Equal(t, []string{"some", "Snake", "Case"}, actual) + actual = SplitSymbol("SomeCamelCase") + require.Equal(t, []string{"Some", "Camel", "Case"}, actual) + actual = SplitSymbol("SomeCamelCaseAPI") + require.Equal(t, []string{"Some", "Camel", "Case", "API"}, actual) + actual = SplitSymbol("some_underscore_case") + require.Equal(t, []string{"some", "underscore", "case"}, actual) + actual = SplitSymbol("SOME_UNDERSCORE_CASE") + require.Equal(t, []string{"SOME", "UNDERSCORE", "CASE"}, actual) + actual = SplitSymbol("ListingDBService") + require.Equal(t, []string{"Listing", "DB", "Service"}, actual) + actual = SplitSymbol("some1") + require.Equal(t, []string{"some1"}, actual) + actual = SplitSymbol("_id") + require.Equal(t, []string{"", "id"}, actual) + actual = SplitSymbol("listingIdSHAs") + require.Equal(t, []string{"listing", "Id", "SHA", "s"}, actual) + actual = SplitSymbol("listingIdSHAsToAdd") + require.Equal(t, []string{"listing", "Id", "SHA", "s", "To", "Add"}, actual) + actual = SplitSymbol("APIv3ProtocolTestService") + require.Equal(t, []string{"API", "v3", "Protocol", "Test", "Service"}, actual) +} + +func TestUpperCamelCase(t *testing.T) { + actual := UpperCamelCase("listingIdSHAs") + require.Equal(t, "ListingIDSHAs", actual) + actual = UpperCamelCase("listingIdSHAsToAdd") + require.Equal(t, "ListingIDSHAsToAdd", actual) +} + +func TestLowerCamelCase(t *testing.T) { + actual := LowerCamelCase("listingIdSHAs") + require.Equal(t, "listingIDSHAs", actual) + actual = LowerCamelCase("listingIdSHAsToAdd") + require.Equal(t, "listingIDSHAsToAdd", actual) +} + +func TestComment(t *testing.T) { + enum := &parser.Enum{ + Comment: strings.Repeat("hello ", 30), + } + actual := Comment(enum) + expected := []string{ + "hello hello hello hello hello hello hello hello hello hello hello hello hello", + "hello hello hello hello hello hello hello hello hello hello hello hello hello", + "hello hello hello hello", + } + require.Equal(t, expected, actual) + enum = &parser.Enum{} + actual = Comment(enum) + require.Nil(t, actual) +} diff --git a/types.go b/types.go new file mode 100644 index 0000000..c644f69 --- /dev/null +++ b/types.go @@ -0,0 +1,35 @@ +package thriftlint + +import ( + "reflect" + + "github.com/alecthomas/go-thrift/parser" +) + +// Types and their supported annotations. +var ( + TypeType = reflect.TypeOf(parser.Type{}) + + ThriftType = reflect.TypeOf(parser.Thrift{}) + + ServiceType = reflect.TypeOf(parser.Service{}) + MethodType = reflect.TypeOf(parser.Method{}) + + EnumType = reflect.TypeOf(parser.Enum{}) + EnumValueType = reflect.TypeOf(parser.EnumValue{}) + + StructType = reflect.TypeOf(parser.Struct{}) + FieldType = reflect.TypeOf(parser.Field{}) + + ConstantType = reflect.TypeOf(parser.Constant{}) + TypedefType = reflect.TypeOf(parser.Typedef{}) +) + +// Attempt to extra positional information from a struct. +func Pos(v interface{}) parser.Pos { + rv := reflect.Indirect(reflect.ValueOf(v)) + if f := rv.FieldByName("Pos"); f.IsValid() { + return f.Interface().(parser.Pos) + } + return parser.Pos{} +}