Migrate dead code detection to Prism #527
Conversation
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
A visitor that always go through `visit` to allow common behavior for all nodes without having to define all the `visit_*` methods. Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
lib/spoom/deadcode.rb
Outdated
| message = result.errors.map do |e| | ||
| "#{e.message} (at #{e.location.start_line}:#{e.location.start_column})." | ||
| end.join(" ") |
There was a problem hiding this comment.
It seems like you are trying to display multiple errors in a single line here, and I think the resulting text ends up being confusing.
How about building something like, which is much easier to read IMO:
Encountered error(s) while parsing foo.rb:
- expected a `)` to close the parameters (at 1:8).
- cannot parse the expression (at 1:8).
- expected an `end` to close the `def` statement (at 1:8).
There was a problem hiding this comment.
The thing is that we do not display the errors straight away, instead we raise and I always been under the impression that exception messages should be single line? wdyt?
There was a problem hiding this comment.
I am not sure if it is a requirement for the message to be a single line. After all, the docs for detailed_message do talk about decorating the first line of the exception message which implies that it might be expecting more than 1 line: https://ruby-doc.org/3.3.0/Exception.html#method-i-detailed_message
It may add the class name of the exception to the end of the first line.
There is another option at your disposal as well, though. You could store the formatted errors in your custom exception, and do the formatting at the site that's doing the handling. You could even encapsulate the formatting in the to_s of the exception class.
There was a problem hiding this comment.
Here you go 35b15d8 👍
By default, `srb` will look for RBIs coming from gems. Since we migrated to Prism, Sorbet is now picking up the RBI shipped with the gem which results in a lot of errors. We can disable this behavior by using Tapioca. See sorbet/sorbet#3332. Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
Replace the dependency on SyntaxTree by Prism.
This PR may be easier to read commit by commit as switching the parser deeply impacted the implementation and required a lot of changes. I used the tests as source of truth.