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

Enhance user experience by providing a linter for Chapel #9003

Closed
LouisJenkinsCS opened this issue Mar 28, 2018 · 19 comments
Closed

Enhance user experience by providing a linter for Chapel #9003

LouisJenkinsCS opened this issue Mar 28, 2018 · 19 comments

Comments

@LouisJenkinsCS
Copy link
Member

LouisJenkinsCS commented Mar 28, 2018

I believe that a linter should be provided for at least one text editor. A linter makes it significantly easier to program for both newcomers and experienced veterans in that it saves time by showing compiler errors and warnings on-demand without needing to manually compile (I.E Lint-On-Save). Although I am inexperienced with the actual creation of linters, after doing some digging around I believe I have discovered a simple example that can serve as the base for development. Using the Atom text editor as an example, providing an extension to the core AtomLinter package may not be too difficult as it seems to boil down to parsing the compiler output into a JSON Object that AtomLinter can process. Besides for other boilerplate code, I believe that this can be done and I do volunteer to create a simple prototype as I am very interested in having it myself.

There are some other issues that I believe would need to be addressed eventually, such as any internal errors that can arise... how should they be represented? Normal errors have the format "$FILE_PATH:$LINE_NUMBER: $ERROR_TYPE: $ERROR_MESSAGE", but would this be enough to catch all such errors and issues? Are there an other formats you'd need to look out for?

Screenshot of Progress:

image

Repository (WIP):

Available here: https://github.com/LouisJenkinsCS/Chapel-Linter

@buddha314
Copy link

There is a Chapel plugin for both Vim and Atom Is that what you mean?

@LouisJenkinsCS
Copy link
Member Author

That's for syntax highlighting, I mean something like this but for Chapel where it shows both warnings and errors that could only be caught via compilation.

@buddha314
Copy link

Oh, yeah. I've used jsonlint quite a bit. Thanks for explaining it.

@LouisJenkinsCS
Copy link
Member Author

LouisJenkinsCS commented Mar 28, 2018

While developing this, I realize that a few additions are needed to improve this further...

  1. The compiler errors should output the start of the column, as currently I need to set the error position to be the start of the line; if there are two or more errors on a single line, how can the user differentiate?
  2. Is there an easy way to obtain potential 'compiler warning' output that shows dangerous behavior but does not constitute an actual error?
  3. Is there a way to force the compiler to evaluate all code paths including what gets removed during DeadCodeElimination, and perhaps have it continue to output all errors despite encountering an error early on.

@LouisJenkinsCS
Copy link
Member Author

I have a good screenshot of the current progress...

image

So far I can have it show compiler errors but again, it only shows a single error so its usefulness is limited although it has a lot of potential. Any thoughts?

@LouisJenkinsCS
Copy link
Member Author

@mppf

Since you're the only one I'm comfortable calling on, do you have any idea on how to make it so that the Chapel compiler can optionally continue showing errors past the first it comes across? I know I can disable DeadCodeElimination through compiler flags to ensure the linter will show errors in 'dead' code, but if it requires changes to the Chapel compiler to make it show all errors do you have any pointers of where I should start.

@LouisJenkinsCS
Copy link
Member Author

To show what I mean by it "not continuing to show errors beyond the first it comes across"...

image

@bradcray
Copy link
Member

You could try using the --ignore-errors flag which tells the compiler to try and push past any errors it encounters and continue. Of course, depending on what the error is, continuing on may cause weird things to happen in the compiler. But for a case like this where the user isn't directly interacting with the compiler, it may give you more of what you need.

I'll mention that, generally speaking, we're trying to not "halt compiler on first error" as much as we traditionally have, but that this has been a lazy and incremental change. That said, I think it's unlikely that we'd ever be comfortable, by default, listing multiple errors across multiple passes. I.e., if there are parse errors, I wouldn't expect future versions of the compiler to continue past parsing without an "I'm brave and can take what you throw at me" flag like --ignore-errors.

@LouisJenkinsCS
Copy link
Member Author

Unfortunate --ignore-errors is a bit too 'strong' as it will repeatedly return from the segfault handler, attempt to continue execution, and then segfault again (example), I believe any internal errors it comes across should result in it halting but it should continue on user errors. Thanks for the suggestion though, could come in handy with some improvements.

@bradcray
Copy link
Member

This might simply be considered a bug to fix in --ignore-errors. Since it wasn't designed to be user-facing (and, I suspect, doesn't get much use), it could just be that nobody's ever bothered to address it. Alternatively, we could have multiple levels of --ignore-errors where one would only continue on user errors, the other on any errors.

@LouisJenkinsCS
Copy link
Member Author

To clarify, I understand why it will halt on parse errors and not continue on to static analysis, but even if a program is syntactically correct, if there is an error in pass A and another error in pass B, only the errors discovered in A will be emitted by the compiler which may catch users by surprise when after they fix it they see emission when the compiler halts on B. Although with the use of --baseline and --no-codegen at least it will be a lot cheaper to rerun the linter multiple times.

@LouisJenkinsCS
Copy link
Member Author

I agree on the need for a variant of --ignore-errors such as --ignore-user-errors, etc. I can report the issue with --ignore-errors where a special case should be added for segfaults.

@mppf
Copy link
Member

mppf commented Mar 30, 2018

I didn't know about --ignore-errors and I suspect nobody would object to simple modifications of it, if you have some adjustments that make it more useful for this purpose.

@LouisJenkinsCS
Copy link
Member Author

LouisJenkinsCS commented Mar 30, 2018

Okay then, current goal to implement the following flags...

  • --ignore-user-errors
  • --stop-after-pass
  • --show-columns && --show-columm-range
  • --ignore-errors segfault patch

@ben-albrecht
Copy link
Member

It would be nice for a linter to catch when user code uses an auto-used module, e.g. use Math.

@LouisJenkinsCS
Copy link
Member Author

I think I would need the ability to query more information from the compiler to support something like this. For example: "Given a specific module X, what are the imports of X?" I could grep for the pattern myself too, but having such support from the compiler would be more resilient to language changes.

@ben-albrecht
Copy link
Member

I think I would need the ability to query more information from the compiler to support something like this.

Agreed. I just wanted to document the desire for that feature.

@mppf
Copy link
Member

mppf commented Feb 3, 2020

I think of this as related to #7295.

@lydia-duncan
Copy link
Member

We have added a linter since this request was made, see: https://chapel-lang.org/docs/latest/tools/chplcheck/chplcheck.html

Thanks especially to @DanilaFe and I believe @jabraham17 for all their hardwork (and I suspect I'm forgetting other names that have contributed to it)

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

No branches or pull requests

7 participants