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

[Fix #5628] Rewrite SpaceInsideStringInterpolation #7228

Merged
merged 3 commits into from Jul 24, 2019

Conversation

buehmann
Copy link
Contributor

This fixes #5628 by rewriting Layout/SpaceInsideStringInterpolation in terms of the infrastructure used by similar SpaceInside* cops: SurroundingSpace and SpaceCorrector.

In order to to that I had to lift some limitations of SurroundingSpace:

  • Previously it assumed that each delimiter token was exactly 1 char wide. It can now handle wider tokens such as #{.
  • Previously it scanned across tab characters when detecting surrounding space, but did not include them in the range to be removed. This mismatch is gone.

In style space there was an overlap in functionality with cop ExtraSpace: Layout/SpaceInsideStringInterpolation no longer removes excess whitespace and leaves that to ExtraSpace. That's why the specs needed to change.

Because of that slight change in functionality I was wondering whether to include this into the "bug fixes" or "changes" section in the change log. For now I just classified this as a bug fix.

Previously tabs and spaces were used to detect offenses but only spaces
were removed. This fixes that mismatch.
`SurroundingSpace` previously assumed that each delimiter token was
exactly 1 char wide. The start of an interpolation `#{` is not.
@buehmann
Copy link
Contributor Author

I am not familiar with jruby. Why do my specs fail there?

@koic
Copy link
Member

koic commented Jul 23, 2019

FYI, this is a known issue of jruby/jruby#4260. The JRuby build matrix does not pass until JRuby 9.2.8.0 release. Therefore, it is still pending to use heredoc for <<~'RUBY'.

- expect_offense(<<~'RUBY')
+ expect_offense(<<-'RUBY'.strip_indent)

Related conversation #7173 (comment)

Rewrite in terms of `SurroundingSpace` and `SpaceCorrector`.
In style `space` there was an overlap in functionality with cop
`ExtraSpace`: This cop no longer removes excess whitespace.
@buehmann buehmann force-pushed the space-inside-string-interpol branch from 6b2a1da to 87cff0c Compare July 24, 2019 05:26
@buehmann
Copy link
Contributor Author

Thank you very much. Workaround applied.

@bbatsov bbatsov merged commit faf8d7e into rubocop:master Jul 24, 2019
@bbatsov
Copy link
Collaborator

bbatsov commented Jul 24, 2019

Nice done! Thanks!

@buehmann buehmann deleted the space-inside-string-interpol branch July 24, 2019 16:17
koic added a commit to koic/rubocop that referenced this pull request Aug 21, 2019
jruby/jruby#4260 has been resolved and
JRuby 9.2.8.0  is available on CircleCI.
https://hub.docker.com/r/circleci/jruby/tags

```console
% docker run --rm -it circleci/jruby:9.2
Unable to find image 'circleci/jruby:9.2' locally
9.2: Pulling from circleci/jruby
9cc2ad81d40d: Pull complete
e6cb98e32a52: Pull complete
ae1b8d879bad: Pull complete
2383fa4462e7: Pull complete
7ac3ce9f2067: Pull complete
ce9a16d8ddcb: Pull complete
b250e79d9c7f: Pull complete
ce3b7f0ecca6: Pull complete
2405779516de: Pull complete
902dc7b355a1: Pull complete
432a6994cb77: Pull complete
66a081bddadb: Pull complete
1576af617cb5: Pull complete
4233b310ce28: Pull complete
51c0ac4a73c0: Pull complete
c43fdb6f1fe8: Pull complete
909d5a0f7f69: Pull complete
940e076596c5: Pull complete
c6abd11f682b: Pull complete
678a31b2fb39: Pull complete
Digest:
sha256:ee43918172710e4871054ce450da6e5f6d66cdeb1fe8397d356742bf142f20a2
Status: Downloaded newer image for circleci/jruby:9.2
$ ruby -v
jruby 9.2.8.0 (2.5.3) 2019-08-12 a1ac7ff OpenJDK 64-Bit Server VM
25.222-b10 on 1.8.0_222-b10 +jit [linux-x86_64]
```

This PR aims to solve the following issues solved by JRuby.

- rubocop#7026 (comment)
- rubocop#7173 (comment)
- rubocop#7229 (comment)
- rubocop#7228 (comment)

It also removes unnecessary `strip_indent`.
bbatsov pushed a commit that referenced this pull request Aug 21, 2019
jruby/jruby#4260 has been resolved and
JRuby 9.2.8.0  is available on CircleCI.
https://hub.docker.com/r/circleci/jruby/tags

```console
% docker run --rm -it circleci/jruby:9.2
Unable to find image 'circleci/jruby:9.2' locally
9.2: Pulling from circleci/jruby
9cc2ad81d40d: Pull complete
e6cb98e32a52: Pull complete
ae1b8d879bad: Pull complete
2383fa4462e7: Pull complete
7ac3ce9f2067: Pull complete
ce9a16d8ddcb: Pull complete
b250e79d9c7f: Pull complete
ce3b7f0ecca6: Pull complete
2405779516de: Pull complete
902dc7b355a1: Pull complete
432a6994cb77: Pull complete
66a081bddadb: Pull complete
1576af617cb5: Pull complete
4233b310ce28: Pull complete
51c0ac4a73c0: Pull complete
c43fdb6f1fe8: Pull complete
909d5a0f7f69: Pull complete
940e076596c5: Pull complete
c6abd11f682b: Pull complete
678a31b2fb39: Pull complete
Digest:
sha256:ee43918172710e4871054ce450da6e5f6d66cdeb1fe8397d356742bf142f20a2
Status: Downloaded newer image for circleci/jruby:9.2
$ ruby -v
jruby 9.2.8.0 (2.5.3) 2019-08-12 a1ac7ff OpenJDK 64-Bit Server VM
25.222-b10 on 1.8.0_222-b10 +jit [linux-x86_64]
```

This PR aims to solve the following issues solved by JRuby.

- #7026 (comment)
- #7173 (comment)
- #7229 (comment)
- #7228 (comment)

It also removes unnecessary `strip_indent`.
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

Successfully merging this pull request may close these issues.

Infinite loop on running rubocop -a on some code with HERE doc
3 participants