Skip to content
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

Original, absolute paths in linter output #169

Open
rnorth opened this issue Dec 5, 2023 · 2 comments
Open

Original, absolute paths in linter output #169

rnorth opened this issue Dec 5, 2023 · 2 comments

Comments

@rnorth
Copy link

rnorth commented Dec 5, 2023

Thanks for this really useful plugin, and it's great to see it is now part of the bufbuild org!

I'd like to propose a tweak to linter output, though I'm not sure how it could be implemented. We're seeing linter output like the following:

Some Protobuf files had lint violations:
..-proto/company/service/v1/schema.proto:52:9:...

(The ..- bit comes about because our proto schemas sit in a ../proto directory adjacent to the gradle submodule where this plugin is being run. I don't think this is relevant to this overall request, though)

The overall path shown, though, is a bit unergonomic: IDEs do not show it as a clickable link, making it quite fiddly to find the erroneous file and line. For IDEs to render the link as clickable, I believe an absolute path is required.

I think it would be a better developer experience if:

  • the paths were absolute, not relative
  • the paths were to the original source files instead of the build/bufbuild/... copies. Obviously only when these are real files on disk rather than protobuf dependencies etc...

The build logs would thus look like:

Some Protobuf files had lint violations:
/Users/me/some/project/proto/company/service/v1/schema.proto:52:9:...

I'm wondering if some kind of transformation of the buf lint output could be applied as a worst case option, or if there's a way to handle this better somewhere else.

bufbuild/buf#125 suggests that buf lint should be capable of emitting absolute paths if it has been given an absolute path as an input.

Has this been considered at some point?

@andrewparmet
Copy link
Collaborator

I have not considered it but this feels like a useful workflow to support.

I'm not sure how this will interact with the symlinking we do for usages with the protobuf-gradle-plugin.

Additionally I may need to undo the symlinking and revert to copying in that case anyways - both to support unprivileged users on Windows (#106) and to allow specifying ignores for image creation that target protobuf dependencies, which is an issue we've had internally but I haven't created a minimal reproducer yet.

@andrewparmet
Copy link
Collaborator

I looked into undoing the symlinking like I mentioned and unfortunately it's not helpful for this use case since we have to provide a workspace [1], so we need (links to) the proto files co-located with the dependencies extracted by the protobuf-gradle-plugin. Now that I think about it it, I may have mentioned undoing symlinking as an obstacle to supporting this, but I don't remember.

I'm open to either of the alternatives you propose; preferably not transforming the output if we can get Buf to emit the non-symlink path. I'm reasonably busy right now so I don't anticipate working on this; I'd be happy to review a PR for this.

[1] An alternative one might consider would be to run buf lint and buf format check against the source set directories themselves for the protobuf-gradle-plugin use case. Historically that did not work (at least for lint) - I recall running into issues with Buf trying to analyze the full build even at the lint step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants