search

facebook / buck2

Build system, successor to Buck
https://buck2.build/
Apache License 2.0
3.59k stars 224 forks source link

Linting #263

Open benbrittain opened 1 year ago

benbrittain commented 1 year ago

There is some discussion on the buck2 discord about best practices with linting and how to scale linting at smaller repo scales vs massive monorepos. The expectation of a massive monorepo tends to be that you have linters run on a CL upload. That works well with infra, but doesn't scale down to early stage open source projects.

Similar to RunInfo/DefaultInfo/InstallInfo, would there be any willingness to support something like a LintInfo that toolchains could use? This would allow folks to leverage the files changed graph that buck2 already has, and allow folks to drop into arbitrary buck2 projects and know how linting works.

thoughtpolice commented 1 year ago

I'm not sure increasing the surface area of the core API is needed for this. See an earlier report I wrote about UX improvements: https://github.com/facebook/buck2/issues/86 (please read my verbose-as-usual follow up comments aside from the original one)

If it's mostly about UX, which I think it is — then if this framework was in place, you could easily write a buck fmt or buck lint or buck fix command, backed by proper BXL wrappers, that can do necessary build introspection, run the tools, and commit the results, etc. You could already do this today, you just wouldn't have the awesome shorthand syntax. I think the question is something like — how does LintInfo differ in semantics from RunInfo? It's a bit odd... That said, this could all be prototyped with BXL, I think.

At small scales, I would say that tracking the files changed in the graph isn't really necessary from a performance POV. "Small" here means "It can fit on one computer", in my mind. Just using git status or git show or whatever and requiring any modified file have the formatter run on it is easy, can be added to your CI pipeline trivially, and ultimately doesn't require much more from buck than it can already do today.

benbrittain commented 1 year ago

I really like #86, I'll admit I've looked at BXL less than I should have. I think it could come really really close to a dedicated linting feature without expanding the core API as you said.

There is some benefit in having it provided by default at the start of setting up a project however. I think that could be solved the some sort of bxl script in prelude and thelint subcommand being set up by default on project init. It'd also let projects remove or totally rework what lint means which keeps flexibility.

thoughtpolice commented 1 year ago

Yeah, thinking back on it this morning, I'm not even entirely convinced by my own argument! It would be a bit unfortunate to have everyone recreate these 2-3 commands over and over through BXL for every user...

Maybe there should be a lint command. But it just shouldn't do much and basically works like RunInfo for a target, it's just expected you're, well, running a linter! So having some more input/thoughts here would be good

benbrittain commented 1 year ago

The biggest benefit of a dedicated lint command would be that it could also take a --fix param for language toolchains that support it. I don't know if the buck2 maintainers are comfortable with the build system modifying files in-tree though, I'm not sure I would be. It would be nice ergonomics though.

cbarrete commented 2 months ago

@thoughtpolice Have you played with BXL for linting? If so, what was your experience like? Have you run into issues like https://github.com/facebook/buck2/issues/748?