Gazelle: initial implementation of rule index #1046
Conversation
(Unit tests will be in the PR to wire this into Resolver). Related bazel-contrib#859
go/tools/gazelle/config/constants.go
Outdated
| type Language int | ||
|
|
||
| const ( | ||
| GoLang Language = iota |
There was a problem hiding this comment.
Public symbols, trivial comments needed...
go/tools/gazelle/resolve/index.go
Outdated
| // Note that a rule may be returned even if visibility restrictions will be | ||
| // be violated. Bazel will give a descriptive error message when a build | ||
| // is attempted. | ||
| func (ix *RuleIndex) findRuleByImport(imp importSpec, lang config.Language, fromRel string) (*ruleRecord, error) { |
There was a problem hiding this comment.
In the presence of multiple vendor folders, we may need to be able to pick the "nearest" in the hierarchy here as well, but it's not clear to me that there is enough information to do so in the structures.
There was a problem hiding this comment.
I've added this logic for Go only (not proto). I'm using the Pkg part of the label as a proxy for the directory in the repository. This won't work in flat mode or with manually written rules, but I'm not sure how much vendoring matters for those cases.
go/tools/gazelle/resolve/index.go
Outdated
| return visibility | ||
| } | ||
|
|
||
| func isVisibleFrom(visibility visibilitySpec, defRel, useRel string) bool { |
There was a problem hiding this comment.
I am not convinced we should be handling visibility, I think it is going to be more surprising than helpful, especially as it's going to be hard to be sure we cover all the visibility cases.
I think the set of cases where visibility is going to help us pick the right one of multiple packages with the same import path are negligible to non existent, far more likely are the cases where we should generate the dep anyway so they get a nice "is not visible" error when they try to build the thing they really wanted.
There was a problem hiding this comment.
You're right, I think I half-realized this when I allowed findRuleByImport to succeed when the only matches available were restricted by visibility.
I've removed all visibility parsing.
* The index no longer handles visibility at all. * Vendoring logic is used to disambiguate imports for Go. * Embedded Go libraries won't be importable from Go, but they can still be imported from proto. * We also do a second looking for proto.
(Unit tests will be in the PR to wire this into Resolver).
Related #859