Validate output file is valid Ruby code about plugin-ruby HOT 16 CLOSED

I don't think that'll be necessary. At the moment the test suite runs rubocop after we've generated the test file, so that should ensure we're generating valid ruby as long as we rely on the test suite.

from plugin-ruby.

@kddeisz The existing test suite doesnt cover all cases currently, and I've hit issues with files that aren't runnable after prettier ruby has processed and updated them

I think it's functionality worth adding until prettier ruby always generates valid outputs for all code bases and we have a more extensive test suite

from plugin-ruby.

The problem is it's going to result in shelling out to two different processes and add a lot of time to the conversion. If we find breaking changes, we should put them into the test suite so that we have more coverage.

from plugin-ruby.

Either way, the most recent PR I'm pushing up should fix a lot of problems we've been having. Also don't want to dismiss your idea out of hand - let's keep talking about it if we keep running into issues.

from plugin-ruby.

I knew I wasn't being crazy; There's a flag for this sort of functionality already:

--debug-check

If you're worried that Prettier will change the correctness of your code, add --debug-check to the command. This will cause Prettier to print an error message if it detects that code correctness might have changed. Note that --write cannot be used with --debug-check.

https://prettier.io/docs/en/cli.html#debug-check

Nothing actionable here, but it just means it'll be easier to create new issues in the future with this flag

from plugin-ruby.

Example when running against rails checked out locally:

master $ yarn prettier --debug-check --plugin=. --parser=ruby "../rails/activesupport/lib/active_support/*.rb"

[etc etc]

../rails/activesupport/lib/active_support/configurable.rb[error] ../rails/activesupport/lib/active_support/configurable.rb: Error: /users/user/Documents/code/prettier-ruby-1/src/ripper.rb:33:in `block in parse': undefined method `[]' for nil:NilClass (NoMethodError)
[error] 	from /users/user/Documents/code/prettier-ruby-1/src/ripper.rb:30:in `tap'
[error] 	from /users/user/Documents/code/prettier-ruby-1/src/ripper.rb:30:in `parse'
[error] 	from /users/user/Documents/code/prettier-ruby-1/src/ripper.rb:228:in `<main>'
[error]
[error]     at Object.module.exports [as parse] (/users/user/Documents/code/prettier-ruby-1/src/parse.js:9:11)
[error]     at Object.parse$2 [as parse] (/users/user/Documents/code/prettier-ruby-1/node_modules/prettier/bin-prettier.js:10641:19)
[error]     at coreFormat (/users/user/Documents/code/prettier-ruby-1/node_modules/prettier/bin-prettier.js:13858:23)
[error]     at format (/users/user/Documents/code/prettier-ruby-1/node_modules/prettier/bin-prettier.js:14117:73)
[error]     at formatWithCursor (/users/user/Documents/code/prettier-ruby-1/node_modules/prettier/bin-prettier.js:14133:12)
[error]     at /users/user/Documents/code/prettier-ruby-1/node_modules/prettier/bin-prettier.js:42401:15
[error]     at Object.format (/users/user/Documents/code/prettier-ruby-1/node_modules/prettier/bin-prettier.js:42420:12)
[error]     at format$1 (/users/user/Documents/code/prettier-ruby-1/node_modules/prettier/bin-prettier.js:43687:27)
[error]     at /users/user/Documents/code/prettier-ruby-1/node_modules/prettier/bin-prettier.js:43965:16
[error]     at /users/user/Documents/code/prettier-ruby-1/node_modules/prettier/bin-prettier.js:43905:14

from plugin-ruby.

This is something the Python formatter Black does:

Also, as a temporary safety measure, Black will check that the reformatted code still produces a valid AST that is equivalent to the original. This slows it down. If you're feeling confident, use --fast.

from plugin-ruby.

Okay. I'm going to reopen this for future discussion as an option.

from plugin-ruby.

Does --debug-check not do what you need?

from plugin-ruby.

I think given some of the issues raised recently about returning invalid code or non-formatting related bugs, having this automated would mean you can trust that the AST you start with is the same AST you're given without having to manually verify every change (which matters a lot on projects like mine which have 134k lines across 2300 files).

from plugin-ruby.

Have you tried --debug-check @NoahTheDuke?

from plugin-ruby.

@j-f1 --debug-check verifies that the AST is equivalent, but the Ruby prettier doesn't honour those semantics.

For instance rewriting if statements into ternaries, do ... end blocks into single line { ... } blocks, or even just adding brackets to method calls results in a different AST in Ruby:

$ prettier --debug-check --plugin=. --parser=ruby test/cases/next.rb
test/cases/next.rb[error] test/cases/next.rb: ast(input) !== ast(prettier(input))

[error] Index:
[error] ===================================================================
[error] ---
[error] +++
[error] @@ -187,20 +187,6 @@
[error]                                "body": [
[error]                                  {
[error] -                                  "type": "paren",
[error] -                                  "body": [
[error] -                                    {
[error] -                                      "type": "stmts",
[error] -                                      "body": [
[error] -                                        {
[error] -                                          "type": "@int",
[error] -                                          "body": "1",
[error] -                                          "start": 7,
[error] -                                          "end": 7
[error] -                                        }
[error] -                                      ],
[error] -                                      "start": 7,
[error] -                                      "end": 7
[error] -                                    }
[error] -                                  ],
[error] +                                  "type": "@int",
[error] +                                  "body": "1",
[error]                                    "start": 7,
[error]                                    "end": 7
[error]
[error] Index:
[error] ===================================================================
[error] ---
[error] +++
[error] @@ -5,3 +5,3 @@
[error]  [].each { next 1 }
[error]
[error] -[].each { next(1) }
[error] +[].each { next 1 }
[error]

from plugin-ruby.

My apologies, I mean that including the --debug-check by default (making it work with --write of course) would help overall, because you'd never accidentally forget to run it; you could "fire and forget" and be assured that using prettier-ruby wouldn't ever damage your code.

EDIT: AlanFoster made my point better for me than I could.

from plugin-ruby.

but the Ruby prettier doesn't honour those semantics

To fix that, I’d suggest wiring up a clean function that normalizes the ASTs before debug-check compares them.

My apologies, I mean that including the --debug-check by default (making it work with --write of course) would help overall, because you'd never accidentally forget to run it; you could "fire and forget" and be assured that using prettier-ruby wouldn't ever damage your code.

I believe the reason we don’t enable it by default is that it’s slower due to having to parse the code twice and it isn’t usually necessary as long as the test suite is good enough.

from plugin-ruby.

(running the test suite against the rails codebase and checking for errors would be sweet)

from plugin-ruby.

Okay I'm going to close this again as it's pretty much stalled. Writing a clean function would require doing a pretty massive refactor and rerunning the parser would be very costly for performance. If there's another idea or PR, let's open up another one as this is starting to spam a lot of folks.

from plugin-ruby.