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
Implement lint command feature #406
Implement lint command feature #406
Conversation
Thanks for opening this PR! 🎉
Since parse errors leads to not including the entities in that file it might result in a type error if any entity in that file is used elsewhere. We can just not type-check if we have parsing errors.
It's the same kind of error as the other so it can be catched (I'll suggest it in the review) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can skip terminal.measure
calls since it doesn't make any sense to print it at the bottom.
There is a linting issue in the CI build, you can run the tests and linting with the make test
command.
…sure, allow for multiple type-errors
I've added a |
It actually detects all type errors this way? 😮 |
It's still limited to one type-error per file 😕, but it does capture the first for each of them |
Actually, I think it might be catching multiple per file. Perhaps it depends on the kind of type-error 🤔 It's catching type-errors from different functions in the same file, but only the first type-error from each function: Mint - Linting
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
░ TYPE ERROR ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░
The return type of a function does not match its type definition.
I was expecting:
Html
Instead it is:
String
Here is the relevant code snippet:
┌──┬ source/components/Exchange.mint:38:3 ─────────────────────────────────────┐
│34│ } │
│35│ } │
│36│ } │
│37│ │
│38│ fun render : Html { │
│39│ │
│40│ "" │
│41│ } │
│42│ } │
└──┴───────────────────────────────────────────────────────────────────────────┘
░ TYPE ERROR ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░
You were trying to assign an incompatible value to the status state.
The type of the state is: String
But the type you are trying to assign to it: Number
Here is where you did the assignment:
┌──┬ source/components/Exchange.mint:23:12 ────────────────────────────────────┐
│19│ next { baseValues = values } │
│20│ } │
│21│ │
│22│ fun updateCompareLocale (values : InputValues) : Promise(Never, Void) { │
│23│ next { compareLocale = 6 } │
│24│ } │
│25│ │
│26│ fun handleSubmit (e : Html.Event) { │
│27│ sequence { │
└──┴───────────────────────────────────────────────────────────────────────────┘
And here is where the state is defined:
┌──┬ source/components/Exchange.mint:16:3 ─────────────────────────────────────┐
│12│ display: flex; │
│13│ } │
│14│ │
│15│ state baseValues : InputValues = InputValues("0", "GBP") │
│16│ state compareLocale : String = "USD" │
│17│ │
│18│ fun updateBaseValues (values : InputValues) : Promise(Never, Void) { │
│19│ next { baseValues = values } │
│20│ } │
└──┴───────────────────────────────────────────────────────────────────────────┘
░ TYPE ERROR ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░
The 1st argument to a function is causing a mismatch.
The function is expecting the 1st argument to be:
Html.Event(
bubbles: Bool,
...
event: Html.NativeEvent)
Instead it is:
String
The type of the function is:
Function(
Html.Event(
bubbles: Bool,
...
event: Html.NativeEvent),
Void)
You tried to call it here:
┌──┬ source/components/Exchange.mint:28:32 ────────────────────────────────────┐
│24│ } │
│25│ │
│26│ fun handleSubmit (e : Html.Event) { │
│27│ sequence { │
│28│ Html.Event.preventDefault("e") │
│29│ │
│30│ if (baseValues.amount == "0") { │
│31│ setStatusMessage(6) │
│32│ } else { │
└──┴───────────────────────────────────────────────────────────────────────────┘ |
I'm also wondering if it's worth formatting the JSON better. So, have each error as an object with separate properties, one for error type, one for line/character number and one for message. I'm not sure there's a simple way to do that, but, just a thought |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just found one issue which is basically the exceptions are printed to STDOUT even if the json
flag is set, if fixed then it's good to go for me 👍
Yeah, it would be great if the errors would contain the source path, line and column numbers but it's a bit tricky to do now, so let's add that as an enchantment for the future. |
As I said before, I have no experience with Crystal, so this could all be completely useless. If anything, it's a start for someone else to pick up and finish off.
This PR addresses #405.
What
Following
compile.cr
as an example, and considering the points raised in #405, I've made an implementation of a new command for Mint:mint lint
.How
--json
, writes the output as an array to the STDOUTmint.json
is logged, but isn't captured within the JSON outputexit 1
if there are any errorsNotes
It would be really useful if all type-errors could be outputted instead of only the first one, again, no idea if this is doable.
Any parse errors cause a type-error (in my test project at least) which is misleading:
Not the end of the world, but worth noting.