No warnings in Atom when using Rubocop in a Docker container because of --stdin

[zedtux]

I'm using a container which embeds rubocop and I'm using it in Atom.

Atom is not displaying any linter messages when opening a Ruby file, but the "Linter Rubocop: Fix file" is actually updating my source file.

Playing around in the source code of linter-rubocop made me remove the concat of --stdin which fixed my issue, and Atom started to show the lint messages.

I guess this is due to the use case of Docker so I would suggest then to add an option in the package which would prevent from using the --stdin flag so that you can keep the current behaviour but allow this use case to work too.

[zedtux]

BTW I could do PR if you'd like to.

[maschwenk]

We always encourage new contributors, have a go and I'll be happy to review 😃

[zedtux]

Do you agree with this change?

[Arcanemagus]

Double check what is going on here, I'm betting that you are only seeing messages for the on-disk contents of the file, not the contents coming in from stdin (the current editor contents).

[maschwenk]

I read somewhere that you can also pass a filename to --std-in and it'll work, I'm not sure how that will affect the functionality of Rubocop in general though. Might lose on-the-fly linting?

[zedtux]

I tried using --stdin with a file path and it's not doing any error, but is not seeing any offences while the file is full of them.

Doing exactly the same without --stdin make it just working.

I truly believe that --stdin has a particular behaviour requiring a kind of pipe with the IDE, which you don't have with Docker, or I don't know how to do it.

[Arcanemagus]

What I meant was two-fold:

• Are you actually linting the live content without that flag
• Does removing that break non-Docker rubocop

Might lose on-the-fly linting?

This is exactly the problem I'm worried about.

[zedtux]

Are you actually linting the live content without that flag

I don't because I think there is no pipes between the Atom instance and Rubocop which leaves within a Docker container.

Does removing that break non-Docker rubocop

Not in the way I implemented it as it is a checkbox that has to be checked to not pass the --stdin flag so that by default, the current behaviour is preserved.

[maschwenk]

I'm curious how you're actually using this? Are you using docker-sync or just mounting the volume?

[maschwenk]

After talking to a colleague, I think you'll definitely lose the on the fly linting because without std-in I'm pretty sure it'll just lint the entire project (although I do see the last argument is the filePath, so I'm not sure why you'd need a file path and std-in).

I need to review some of this stuff more, until then I hope the Docker solution is working for you. Don't want to rush into this till I understand it better.

[zedtux]

Are you using docker-sync or just mounting the volume?

I'm using docker-sync.

After talking to a colleague, I think you'll definitely lose the on the fly linting because without std-in I'm pretty sure it'll just lint the entire project.

Well with the tests I did, I can tell that it is only linting the current file, not the entire project. I did a Docker image for Rubocop, and in the repo you can see that I'm telling people to create a bash script which is basically ignoring the --stdin flag, and then run the container.

This PR is actually solving the issue at the root.

Honestly I don't know what is the difference between asking rubocop to parse the given file, or parse the stdin, but when I running it manually (so using docker run, passing the same arguments, I can see the generated JSON which shows the amount of parsed files, which is of 1, and all the offences found.

[maschwenk]

I appreciate the thorough description, I'm at work right now so I need to look at this later more deeply to make sure I understand it

[zedtux]

Short update: I use docker-sync in my project, but not for Rubocop, sorry. I'm using a Docker volume to mount the /Users folder (lazy trick in order to handle the given path ... could be improved) and from here, Rubocop can access the file to be linted.

Please feel free to ask me whatever can help you to understand the use case, I'm available to check/fix/update anything to get this working :)

[zedtux]

Just to give a little bit more details, here are the tests I did manually in a terminal (same commands than Atom is running in the end, just to see the output):

With the --stdin:

docker run --rm -v /Users:/Users --workdir / zedtux/docker-rubocop:0.50.0 --cache false --force-exclusion --format json --display-style-guide --stdin /Users/guillaumeh/Developments/company/project/app/models/reporting_document_template.rb
{"metadata":{"rubocop_version":"0.50.0","ruby_engine":"ruby","ruby_version":"2.4.2","ruby_patchlevel":"198","ruby_platform":"x86_64-linux"},"files":[{"path":"sers/guillaumeh/Developments/company/project/app/models/reporting_document_template.rb","offenses":[]}],"summary":{"offense_count":0,"target_file_count":1,"inspected_file_count":1}}%

Without the --stdin:

docker run --rm -v /Users:/Users --workdir / zedtux/docker-rubocop:0.50.0 --cache false --force-exclusion --format json --display-style-guide /Users/guillaumeh/Developments/company/project/app/models/reporting_document_template.rb
{"metadata":{"rubocop_version":"0.50.0","ruby_engine":"ruby","ruby_version":"2.4.2","ruby_patchlevel":"198","ruby_platform":"x86_64-linux"},"files":[{"path":"sers/guillaumeh/Developments/company/project/app/models/reporting_document_template.rb","offenses":[{"severity":"convention","message":"Extra empty line detected at class body beginning. (https://github.com/bbatsov/ruby-style-guide#empty-lines-around-bodies)","cop_name":"Layout/EmptyLinesAroundClassBody","corrected":false,"location":{"line":2,"column":1,"length":1}},{"severity":"convention","message":"Line is too long. [93/80] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)","cop_name":"Metrics/LineLength","corrected":false,"location":{"line":20,"column":81,"length":13}},{"severity":"convention","message":"Line is too long. [90/80] (https://github.com/bbatsov/ruby-style-guide#80-character-limits)","cop_name":"Metrics/LineLength","corrected":false,"location":{"line":30,"column":81,"length":10}}]}],"summary":{"offense_count":3,"target_file_count":1,"inspected_file_count":1}}%

[Arcanemagus]

You need to find some way of passing the stdin stream to the docker process, not just adding the --stdin flag. A few other people have reported success creating a wrapper script that handles this for them: https://github.com/AtomLinter/linter-phpcs/issues/299#issuecomment-338346850

[quolpr]

If I strip absolute path(with ${arg#$PWD/}) it begins work:

#!/bin/bash

CMD_ARGS=""
for arg in $@
do
if [ -f "$arg" ]
then
  CMD_ARGS="$CMD_ARGS ${arg#$PWD/}"
else
  CMD_ARGS="$CMD_ARGS $arg"
fi
done

docker exec -i YOUR_CONTAINER rubocop $CMD_ARGS

[quolpr]

Btw, why are you using absolute path? 🤔

[zedtux]

@quolpr https://github.com/AtomLinter/linter-rubocop/issues/245#issuecomment-335555831

[quolpr]

@zedtux so you can run rubocop without mounting /Users

[zedtux]

Okay thank you @quolpr I will check that then. 👍

[Arcanemagus]

Marking this as closed since this is way outside the scope of this package and is (somewhat) easily solved by creating a wrapper script.

[zedtux]

Yeah but it's boring to create a wrapper script ... and has to be repeated by all developers using Docker 👎

[zedtux]

Or at least something in the README.md people can copy/past and don't wast time.

[Arcanemagus]

@zedtux Across dozens of repos and millions of downloads in the AtomLinter org there are maybe 10 issues talking about Docker support, so making large changes to support it isn't exactly a high priority.

That being said, your #246 PR is a perfectly fine addition, I just recently reviewed it with some wording changes 😉.