-
Notifications
You must be signed in to change notification settings - Fork 1
Feat: add protobuf lint #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
639ab43
fix: barry quick fix, 2025-06-24 22:58:25
kooksee a57b6e5
fix: barry quick fix, 2025-06-25 10:34:29
kooksee 9d9eab6
fix: barry quick fix, 2025-06-25 10:40:20
kooksee 37c5058
fix: barry quick fix, 2025-06-25 11:12:13
kooksee 0a1089c
fix: barry quick fix, 2025-06-25 11:14:59
kooksee a51f4f4
fix: barry quick fix, 2025-06-25 11:30:57
kooksee bd61d7c
fix: barry quick fix, 2025-06-25 12:06:38
kooksee 834f489
fix: barry quick fix, 2025-06-25 14:09:27
kooksee File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| package linters | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "fmt" | ||
| "github.com/samber/lo" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/googleapis/api-linter/lint" | ||
| ) | ||
|
|
||
| // formatGitHubActionOutput returns lint errors in GitHub actions format. | ||
| func formatGitHubActionOutput(responses []lint.Response) []byte { | ||
| var buf bytes.Buffer | ||
| for _, response := range responses { | ||
| for _, problem := range response.Problems { | ||
| // lint example: | ||
| // ::error file={name},line={line},endLine={endLine},title={title}::{message} | ||
| // https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-error-message | ||
|
|
||
| fmt.Println(lo.Must(filepath.Abs(response.FilePath))) | ||
| fmt.Fprintf(&buf, "::error file=%s", response.FilePath) | ||
| if problem.Location != nil { | ||
| // Some findings are *line level* and only have start positions but no | ||
| // starting column. Construct a switch fallthrough to emit as many of | ||
| // the location indicators are included. | ||
| switch len(problem.Location.Span) { | ||
| case 4: | ||
| fmt.Fprintf(&buf, ",endColumn=%d", problem.Location.Span[3]) | ||
| fallthrough | ||
| case 3: | ||
| fmt.Fprintf(&buf, ",endLine=%d", problem.Location.Span[2]) | ||
| fallthrough | ||
| case 2: | ||
| fmt.Fprintf(&buf, ",col=%d", problem.Location.Span[1]) | ||
| fallthrough | ||
| case 1: | ||
| fmt.Fprintf(&buf, ",line=%d", problem.Location.Span[0]) | ||
| } | ||
| } | ||
|
|
||
| // GitHub uses :: as control characters (which are also used to delimit | ||
| // Linter rules. In order to prevent confusion, replace the double colon | ||
| // with two Armenian full stops which are indistinguishable to my eye. | ||
| runeThatLooksLikeTwoColonsButIsActuallyTwoArmenianFullStops := "։։" | ||
| title := strings.ReplaceAll(string(problem.RuleID), "::", runeThatLooksLikeTwoColonsButIsActuallyTwoArmenianFullStops) | ||
|
Comment on lines
+46
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| message := strings.ReplaceAll(problem.Message, "\n", "\\n") | ||
| uri := problem.GetRuleURI() | ||
| if uri != "" { | ||
| message += "\\n\\n" + uri | ||
| } | ||
| fmt.Fprintf(&buf, ",title=%s::%s\n", title, message) | ||
| } | ||
| } | ||
|
|
||
| return buf.Bytes() | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,209 @@ | ||
| package linters | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "errors" | ||
| "fmt" | ||
| "github.com/samber/lo" | ||
| "os" | ||
| "strings" | ||
| "sync" | ||
|
|
||
| "github.com/googleapis/api-linter/lint" | ||
| "github.com/jhump/protoreflect/desc/protoparse" | ||
| "github.com/pubgo/protobuild/internal/typex" | ||
| "github.com/urfave/cli/v3" | ||
| "gopkg.in/yaml.v3" | ||
| ) | ||
|
|
||
| type CliArgs struct { | ||
| //FormatType string | ||
| //ProtoImportPaths []string | ||
| EnabledRules []string | ||
| DisabledRules []string | ||
| ListRulesFlag bool | ||
| DebugFlag bool | ||
| //IgnoreCommentDisablesFlag bool | ||
| } | ||
|
|
||
| func NewCli() (*CliArgs, typex.Flags) { | ||
| var cliArgs CliArgs | ||
|
|
||
| return &cliArgs, typex.Flags{ | ||
| //&cli.BoolFlag{ | ||
| // Name: "ignore-comment-disables", | ||
| // Usage: "If set to true, disable comments will be ignored.\nThis is helpful when strict enforcement of AIPs are necessary and\nproto definitions should not be able to disable checks.", | ||
| // Value: false, | ||
| // Destination: &cliArgs.IgnoreCommentDisablesFlag, | ||
| //}, | ||
|
|
||
| &cli.BoolFlag{ | ||
| Name: "debug", | ||
| Usage: "Run in debug mode. Panics will print stack.", | ||
| Value: false, | ||
| Destination: &cliArgs.DebugFlag, | ||
| }, | ||
|
|
||
| &cli.BoolFlag{ | ||
| Name: "list-rules", | ||
| Usage: "Print the rules and exit. Honors the output-format flag.", | ||
| Value: false, | ||
| Destination: &cliArgs.ListRulesFlag, | ||
| }, | ||
|
|
||
| //&cli.StringFlag{ | ||
| // Name: "output-format", | ||
| // Usage: "The format of the linting results.\nSupported formats include \"yaml\", \"json\",\"github\" and \"summary\" table.\nYAML is the default.", | ||
| // Aliases: []string{"f"}, | ||
| // Value: "", | ||
| // Destination: &cliArgs.FormatType, | ||
| //}, | ||
|
|
||
| //&cli.StringSliceFlag{ | ||
| // Name: "proto-path", | ||
| // Usage: "The folder for searching proto imports.\\nMay be specified multiple times; directories will be searched in order.\\nThe current working directory is always used.", | ||
| // Aliases: []string{"I"}, | ||
| // Value: nil, | ||
| // Destination: &cliArgs.ProtoImportPaths, | ||
| //}, | ||
|
|
||
| //&cli.StringSliceFlag{ | ||
| // Name: "enable-rule", | ||
| // Usage: "Enable a rule with the given name.\nMay be specified multiple times.", | ||
| // Value: nil, | ||
| // Destination: &cliArgs.EnabledRules, | ||
| //}, | ||
| // | ||
| //&cli.StringSliceFlag{ | ||
| // Name: "disable-rule", | ||
| // Usage: "Disable a rule with the given name.\nMay be specified multiple times.", | ||
| // Value: nil, | ||
| // Destination: &cliArgs.DisabledRules, | ||
| //}, | ||
| } | ||
|
|
||
| } | ||
|
|
||
| type LinterConfig struct { | ||
| Rules lint.Config `yaml:"rules,omitempty" hash:"-"` | ||
| FormatType string `yaml:"format_type"` | ||
| IgnoreCommentDisablesFlag bool `yaml:"ignore_comment_disables_flag"` | ||
| } | ||
|
|
||
| func Linter(c *CliArgs, config LinterConfig, protoImportPaths []string, protoFiles []string) error { | ||
| if c.ListRulesFlag { | ||
| return outputRules(config.FormatType) | ||
| } | ||
|
|
||
| // Pre-check if there are files to lint. | ||
| if len(protoFiles) == 0 { | ||
| return fmt.Errorf("no file to lint") | ||
| } | ||
|
|
||
| rules := lint.Configs{config.Rules} | ||
|
|
||
| // Add configs for the enabled rules. | ||
| rules = append(rules, lint.Config{EnabledRules: c.EnabledRules}) | ||
| rules = append(rules, lint.Config{DisabledRules: c.DisabledRules}) | ||
|
|
||
| var errorsWithPos []protoparse.ErrorWithPos | ||
| var lock sync.Mutex | ||
| // Parse proto files into `protoreflect` file descriptors. | ||
| p := protoparse.Parser{ | ||
| ImportPaths: append(protoImportPaths, "."), | ||
| IncludeSourceCodeInfo: true, | ||
| ErrorReporter: func(errorWithPos protoparse.ErrorWithPos) error { | ||
| // Protoparse isn't concurrent right now but just to be safe for the future. | ||
| lock.Lock() | ||
| errorsWithPos = append(errorsWithPos, errorWithPos) | ||
| lock.Unlock() | ||
| // Continue parsing. The error returned will be protoparse.ErrInvalidSource. | ||
| return nil | ||
| }, | ||
| } | ||
|
|
||
| var err error | ||
| // Resolve file absolute paths to relative ones. | ||
| // Using supplied import paths first. | ||
| if len(protoImportPaths) > 0 { | ||
| protoFiles, err = protoparse.ResolveFilenames(protoImportPaths, protoFiles...) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
| // Then resolve again against ".", the local directory. | ||
| // This is necessary because ResolveFilenames won't resolve a path if it | ||
| // relative to *at least one* of the given import paths, which can result | ||
| // in duplicate file parsing and compilation errors, as seen in #1465 and | ||
| // #1471. So we resolve against local (default) and flag specified import | ||
| // paths separately. | ||
| protoFiles, err = protoparse.ResolveFilenames([]string{"."}, protoFiles...) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| fd, err := p.ParseFiles(protoFiles...) | ||
| if err != nil { | ||
| if err == protoparse.ErrInvalidSource { | ||
| if len(errorsWithPos) == 0 { | ||
| return errors.New("got protoparse.ErrInvalidSource but no ErrorWithPos errors") | ||
| } | ||
| // TODO: There's multiple ways to deal with this but this prints all the errors at least | ||
| errStrings := make([]string, len(errorsWithPos)) | ||
| for i, errorWithPos := range errorsWithPos { | ||
| errStrings[i] = errorWithPos.Error() | ||
| } | ||
| return errors.New(strings.Join(errStrings, "\n")) | ||
| } | ||
| return err | ||
| } | ||
|
|
||
| // Create a Linter to lint the file descriptors. | ||
| l := lint.New(globalRules, rules, lint.Debug(c.DebugFlag), lint.IgnoreCommentDisables(config.IgnoreCommentDisablesFlag)) | ||
| results, err := l.LintProtos(fd...) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Determine the format for printing the results. | ||
| // YAML format is the default. | ||
| marshal := getOutputFormatFunc(config.FormatType) | ||
|
|
||
| // Print the results. | ||
| b, err := marshal(results) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| fmt.Println(string(b)) | ||
|
|
||
| filterResults := lo.Filter(results, func(item lint.Response, index int) bool { return len(item.Problems) > 0 }) | ||
| if len(filterResults) > 0 { | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| var outputFormatFuncs = map[string]formatFunc{ | ||
| "yaml": yaml.Marshal, | ||
| "yml": yaml.Marshal, | ||
| "json": json.Marshal, | ||
| "github": func(i interface{}) ([]byte, error) { | ||
| switch v := i.(type) { | ||
| case []lint.Response: | ||
| return formatGitHubActionOutput(v), nil | ||
| default: | ||
| return json.Marshal(v) | ||
| } | ||
| }, | ||
| } | ||
|
|
||
| type formatFunc func(interface{}) ([]byte, error) | ||
|
|
||
| func getOutputFormatFunc(formatType string) formatFunc { | ||
| if f, found := outputFormatFuncs[strings.ToLower(formatType)]; found { | ||
| return f | ||
| } | ||
| return yaml.Marshal | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| package linters | ||
|
|
||
| import ( | ||
| "github.com/googleapis/api-linter/rules" | ||
| "log" | ||
| "os" | ||
| "sort" | ||
|
|
||
| "github.com/googleapis/api-linter/lint" | ||
| ) | ||
|
|
||
| var ( | ||
| globalRules = lint.NewRuleRegistry() | ||
| ) | ||
|
|
||
| func init() { | ||
| if err := rules.Add(globalRules); err != nil { | ||
| log.Fatalf("error when registering rules: %v", err) | ||
| } | ||
| } | ||
|
|
||
| type ( | ||
| listedRule struct { | ||
| Name lint.RuleName | ||
| } | ||
| listedRules []listedRule | ||
| listedRulesByName []listedRule | ||
| ) | ||
|
|
||
| func (a listedRulesByName) Len() int { return len(a) } | ||
| func (a listedRulesByName) Less(i, j int) bool { return a[i].Name < a[j].Name } | ||
| func (a listedRulesByName) Swap(i, j int) { a[i], a[j] = a[j], a[i] } | ||
|
|
||
| func outputRules(formatType string) error { | ||
| rules := listedRules{} | ||
| for id := range globalRules { | ||
| rules = append(rules, listedRule{ | ||
| Name: id, | ||
| }) | ||
| } | ||
|
|
||
| sort.Sort(listedRulesByName(rules)) | ||
|
|
||
| // Determine the format for printing the results. | ||
| // YAML format is the default. | ||
| marshal := getOutputFormatFunc(formatType) | ||
|
|
||
| // Print the results. | ||
| b, err := marshal(rules) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| w := os.Stdout | ||
| if _, err = w.Write(b); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return nil | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using
slogfor logging instead offmt.Println. It provides structured logging and better control over log levels.