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

Custom lint ? #3471

Open
giltho opened this issue May 9, 2020 · 8 comments
Open

Custom lint ? #3471

giltho opened this issue May 9, 2020 · 8 comments

Comments

@giltho
Copy link

giltho commented May 9, 2020

Hi!

Desired Behavior

Hello, it seems that the current system makes it impossible to run a custom promotable linting action, as it is possible for ppx.

(library
   (name ...)
   (lint (pps ppx_yojson_conv))

will let you run dune build @lint and then dune promote

In the same manner, I would expect

(library
  (name ...)
  (lint (action (echo "test")))

to let produce promotable files where every one of my ml and mli file have been overridden by files containing just "test".

I know it's happening in preprocessing.ml > lint_module > | Action where the target is set to None and no promote_correction is happening, but I haven't had time to dive deeper into it to make it work yet

@ghost
Copy link

ghost commented May 12, 2020

You can, but the protocol is secret :)

If you generate a file called file.ml.lint-corrected, dune will automatically offer it as a correction.

@giltho
Copy link
Author

giltho commented May 12, 2020

Unfortunately that does not seem to work :/
That was also my first thought given that this is how it works internally when linting with a ppx.

However, it seems that the definition of the promotion is only done in the Pps branch of the lint_module pattern_matching (see here)

I made a small repro here : https://github.com/giltho/dune-repro-no-lint

I don't know what should be the right way of doing that though, writing to .lint-corrected, or doing the same thing as for preprocessing, i.e. using stdout (https://dune.readthedocs.io/en/stable/concepts.html#preprocessing-with-actions).
Given that lint works in the exact same way as preprocessing other wise (only takes an input-file parameter, and no target), I would expect the stdout solution

@ghost
Copy link

ghost commented May 13, 2020

However, it seems that the definition of the promotion is only done in the Pps branch of the lint_module pattern_matching (see here)

Ah, indeed. I forgot about this.

I don't know what should be the right way of doing that though, writing to .lint-corrected, or doing the same thing as for preprocessing, i.e. using stdout (https://dune.readthedocs.io/en/stable/concepts.html#preprocessing-with-actions).

I suppose that would work. It's a breaking change though so we'd have to handle it carefully.

@NathanReb has been preparing a new improved "linting pipeline" so that different linters can work well together. So any such change should be considered in the context of this work or at least postponed so that we don't go in different directions.

BTW, if you'd like to help on this front that would be welcome :)

@giltho
Copy link
Author

giltho commented May 13, 2020

Sure I'd love to help with it :) And it's clearly not an emergency so it can wait for the linting pipeline rework to be done

@ghost
Copy link

ghost commented May 13, 2020

Great :)

If you'd like to help us improve the linting process in general, we can plan a chat between the three of us so that we can show you our plan and discuss how you can help, or if you'd rather focus on the idea of having lint action produce a correction, we'll come back to you once we have the new linting pipeline in place. Just let me know, thanks!

@giltho
Copy link
Author

giltho commented May 13, 2020

Although I'm interested, I already have too much to do on other projects already :/ I think it would be counter productive for me to participate in the linting pipeline improvement. But do ping me once it is done, or almost, so that I can help with that part :)

@ghost
Copy link

ghost commented May 14, 2020

Sounds good!

@Kakadu
Copy link
Contributor

Kakadu commented Jul 29, 2021

Folks, do you think that specifying linter command should be done outside dune rules, from command line? Imagine the following situation: I force to use dune for homework in my FP course and want to setup a linter. Students remove linting lines from dune files and I'm obliged to force right dune files administratively, instead simply rinning my linter in CI on all code.

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