Core skeleton #1
Conversation
More rules can be now added to it.
lint/context.go
Outdated
| } | ||
|
|
||
| // DescriptorSource returns a `DescriptorSource` if available; otherwise, | ||
| // returns (nil, ErrSourceInfoNotAvailable). |
There was a problem hiding this comment.
I think this comment is out of date.
There was a problem hiding this comment.
Can you update it? Thanks!
lint/context.go
Outdated
|
|
||
| // Context provides additional information for a rule to perform linting. | ||
| type Context struct { | ||
| context context.Context |
There was a problem hiding this comment.
Why is this here? I don't see it being used anywhere.
There was a problem hiding this comment.
As discussed offline, this should be removed.
cmd/protoc-gen-api_lint/main.go
Outdated
|
|
||
| type linter struct { | ||
| reader io.Reader | ||
| writer io.Writer |
There was a problem hiding this comment.
It looks like this is never used, but instead the problems are printed via log.Printf. Is this intentional? And if so, let's get rid of this.
There was a problem hiding this comment.
Can you rewrite this protoc plugin using this helper function? Thanks!
rules/checkers.go
Outdated
| problems []lint.Problem | ||
| } | ||
|
|
||
| func (c *checkers) Lint(rule lint.Rule, req lint.Request) (lint.Response, error) { |
There was a problem hiding this comment.
Oops, the rule should be set: c.rule = rule, and we need tests to catch it.
protovisit/proto_visit.go
Outdated
| } | ||
|
|
||
| // MessageVisitor defines how to travel in a message. | ||
| type MessageVisitor interface { |
There was a problem hiding this comment.
The purpose of this interface is still not really clear to me.
protovisit/proto_visit.go
Outdated
|
|
||
| // MessageVisiting defines a collection of functions that can be applied to a message and | ||
| // its elements. | ||
| type MessageVisiting interface { |
There was a problem hiding this comment.
This still looks like a Visitor. That is, it's performing some operation on the underlying struct hierarchy. I think calling it Visiting might be confusing.
lint/request.go
Outdated
| // ProtoFile returns a `FileDescriptor` when the rule's `FileTypes` | ||
| // contains `ProtoFile`. | ||
| ProtoFile() protoreflect.FileDescriptor | ||
| Context() Context |
There was a problem hiding this comment.
As discussed in person, can we remove the Context struct, as well as the context.Context member, and move the DescriptorSource to this.
There was a problem hiding this comment.
Yes, please. Thanks!
lint/request.go
Outdated
| type Request interface { | ||
| // ProtoFile returns a `FileDescriptor` when the rule's `FileTypes` | ||
| // contains `ProtoFile`. | ||
| ProtoFile() protoreflect.FileDescriptor |
There was a problem hiding this comment.
Are we considering cases where a rule might need visibility into multiple files?
There was a problem hiding this comment.
Do you mean the proto files that it depends, which can be accessed from FileDescriptor.Imports()? Or other types of files, which should have their own getter, like OnePlatformConfigFile()?
lint/context.go
Outdated
| import "context" | ||
|
|
||
| // Context provides additional information for a rule to perform linting. | ||
| type Context struct { |
There was a problem hiding this comment.
@mingzhi because of the fact that the Request struct is passed to the rules, I think Context is superfluous. Any additional "context" that we want can go directly in the Request struct, grouped into more cohesive structs if it makes sense. In other words, I think our Request struct is equivalent to ESLint's context object.
Changed StartLocation to a factory
remove cmd, we will rewrite it later
Removed context and updated tests for DescriptorSource
…with invalid number of elements.
Removed unused error
Added more tests and made `NewDescriptorSource` private.
Added `errInvalidSpan`
* Add =disabled to 0203/immutable.md instructions * Add =disabled to 0203/input-only.md instructions * Add =disabled to 0203/optional-conflict.md * Add =disabled to 0203/optional-consistency.md instructions * Add =disabled to 0203/optional.md instructions * Add =disabled to 0203/output-only.md instructions
This pull request contains the core functionalities of creating and registering a lint rule. Two rule examples are included; however, tests are needed for both rules. The next step will be to add more rules.