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

Wildcard excludes do not work if running within Git Bash #7190

Closed
amyspark opened this issue Jun 30, 2019 · 8 comments
Closed

Wildcard excludes do not work if running within Git Bash #7190

amyspark opened this issue Jun 30, 2019 · 8 comments

Comments

@amyspark
Copy link

Hey all,

I've run into a bug when using Rubocop with bash (Git Bash) under Visual Studio Code.

This is the relevant portion of .rubocop.yml:

require:
  - rubocop-rails
  - rubocop-rspec

Rails:
  Enabled: true

AllCops:
  Exclude:
    - 'app/channels/**/*'
    - 'app/jobs/**/*'
    - 'app/mailers/**/*'
    - 'app/views/**/*'
    - 'bin/*'
    - 'config/initializers/**/*'
    - 'db/schema.rb'
    - 'log/**/*'
    - 'public/**/*'
    - 'script/**/*'
    - 'storage/**/*'
    - 'vendor/**/*'

Layout/AlignArray:
  Enabled: true

Layout/AlignHash:
  Enabled: true

Layout/AlignParameters:
  Enabled: true

Layout/ClosingParenthesisIndentation:
  Enabled: true

Layout/EndAlignment:
  AutoCorrect: true
  EnforcedStyleAlignWith: start_of_line

Layout/IndentFirstArgument:
  Enabled: true

Layout/IndentFirstArrayElement:
  Enabled: true

Layout/IndentFirstHashElement:
  Enabled: true

Layout/IndentFirstParameter:
  Enabled: true

Layout/MultilineArrayLineBreaks:
  Enabled: true

Layout/MultilineHashBraceLayout:
  Enabled: true

Layout/MultilineHashKeyLineBreaks:
  Enabled: true

Layout/MultilineMethodArgumentLineBreaks:
  Enabled: true

Metrics/BlockLength:
  Max: 80
  ExcludedMethods:
    - RSpec.describe

Metrics/LineLength:
  AutoCorrect: true
  Exclude:
    - 'Gemfile'
    - 'Rakefile'
    - 'spec/rails_helper.rb'
    - 'spec/spec_helper.rb'

Rails/FilePath:
  EnforcedStyle: slashes

RSpec/HookArgument:
  EnforcedStyle: each

RSpec/ImplicitExpect:
  EnforcedStyle: should

RSpec/MultipleExpectations:
  Max: 2

Style/BlockComments:
  Exclude:
    - 'spec/spec_helper.rb'

Style/Documentation:
  Exclude:
    - 'config/application.rb'
    - 'db/migrate/**/*'

Style/FrozenStringLiteralComment:
  Exclude:
    - 'Gemfile'
    - 'Rakefile'
    - 'config.ru'
    - 'spec/rails_helper.rb'
    - 'spec/spec_helper.rb'

Expected behavior

When running rubocop -a, it doesn't check excluded files.

Actual behavior

Rubocop checks all paths including the excluded entries:

$ rubocop -a
Inspecting 112 files
............................................C........CCCCCCCCCCC.................................C............C.

Offenses:

config/application.rb:23:3: C: Style/Documentation: Missing top-level class documentation comment.
  class Application < Rails::Application
  ^^^^^
db/migrate/20190606220116_create_languages.rb:3:1: C: Style/Documentation: Missing top-level class documentation comment.
class CreateLanguages < ActiveRecord::Migration[5.2]
^^^^^
db/migrate/20190606220200_create_countries.rb:3:1: C: Style/Documentation: Missing top-level class documentation comment.
class CreateCountries < ActiveRecord::Migration[5.2]
^^^^^
db/migrate/20190606220320_create_author_types.rb:3:1: C: Style/Documentation: Missing top-level class documentation comment.
class CreateAuthorTypes < ActiveRecord::Migration[5.2]
^^^^^
db/migrate/20190606220834_create_project_types.rb:3:1: C: Style/Documentation: Missing top-level class documentation comment.
class CreateProjectTypes < ActiveRecord::Migration[5.2]
^^^^^
db/migrate/20190606221119_create_product_types.rb:3:1: C: Style/Documentation: Missing top-level class documentation comment.
class CreateProductTypes < ActiveRecord::Migration[5.2]
^^^^^
db/migrate/20190606221403_create_areas.rb:3:1: C: Style/Documentation: Missing top-level class documentation comment.
class CreateAreas < ActiveRecord::Migration[5.2]
^^^^^
db/migrate/20190606221457_create_sectors.rb:3:1: C: Style/Documentation: Missing top-level class documentation comment.
class CreateSectors < ActiveRecord::Migration[5.2]
^^^^^
db/migrate/20190606222535_create_disciplines.rb:3:1: C: Style/Documentation: Missing top-level class documentation comment.
class CreateDisciplines < ActiveRecord::Migration[5.2]
^^^^^
db/migrate/20190610205506_create_products.rb:3:1: C: Style/Documentation: Missing top-level class documentation comment.
class CreateProducts < ActiveRecord::Migration[5.2]
^^^^^
db/migrate/20190612214733_create_articles.rb:3:1: C: Style/Documentation: Missing top-level class documentation comment.
class CreateArticles < ActiveRecord::Migration[5.2]
^^^^^
db/migrate/20190618133227_create_product_disciplines.rb:3:1: C: Style/Documentation: Missing top-level class documentation comment.
class CreateProductDisciplines < ActiveRecord::Migration[5.2]
^^^^^
spec/rails_helper.rb:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
# This file is copied to spec/ when you run 'rails generate rspec:install'
^
spec/rails_helper.rb:60:101: C: Metrics/LineLength: Line is too long. [105/100]
  # start by truncating all the tables but then use the faster transaction strategy the rest of the time.
                                                                                                    ^^^^^
spec/spec_helper.rb:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
# This file was generated by the `rails generate rspec:install` command. Conventionally, all
^
spec/spec_helper.rb:54:1: C: [Corrected] Style/BlockComments: Do not use block comments.
=begin ...
^^^^^^
spec/spec_helper.rb:100:1: C: [Corrected] Layout/CommentIndentation: Incorrect indentation detected (column 0 instead of 2).
#   Kernel.srand config.seed
^^^^^^^^^^^^^^^^^^^^^^^^^^^^

112 files inspected, 17 offenses detected, 4 offenses corrected

This still happens when running Rubocop through a CMD instance (Git Bash -> cmd -> rubocop -a), so I believe there's a path construction issue somewhere.

Steps to reproduce the problem

  1. Install Rubocop and Git Bash.
  2. Configure VS Code to use Git Bash's bash by setting terminal.integrated.shell.windows to d:\\Program Files\\Git\\bin\\bash.exe.
  3. Set up a project with excluded paths. Make sure the excluded paths have issues.
  4. Run Rubocop.

RuboCop version

0.71.0 (using Parser 2.6.3.0, running on ruby 2.5.5 x64-mingw32)

@jonas054
Copy link
Collaborator

jonas054 commented Jul 7, 2019

  1. Run Rubocop.

This step is unclear to me. What do I do, exactly?

@amyspark
Copy link
Author

amyspark commented Jul 9, 2019

Apologies @jonas054 ! By "running Rubocop", I meant the following steps. It's probably way excessive, but it will help match my project layout.

  1. Set up a Rails project with rails new test --api -T.
  2. In the Gemfile, add gem rails-rspec. Run bundle and then rails rspec:install.
  3. Copy the .rubocop.yml file I've attached in this issue.
  4. Run rubocop -a on the project folder.

As is, it will throw errors in the spec folder, which should not happen. It's also the same if you add migrations, Rubocop wants me to document them even though I've explicitly excluded the folder.

@jonas054
Copy link
Collaborator

I have not been able to reproduce the problem. I think we need a more minimal reproduction description:

  • a minimal .rubocop.yml
  • can we boil it down to a single file, e.g. config/application.rb?
  • does it occur with --only Style/Documentation?
  • is it only with -a?
  • does it occur only in VS Code or can it be reproduced just running in a Git Bash window?

You said rails-rspec, but meant rubocop-rspec, right?

@amyspark
Copy link
Author

Hey @jonas054 , I'll reply on each point.

a minimal .rubocop.yml

I can't minimize it, as it will introduce additional noise on the output.

can we boil it down to a single file, e.g. config/application.rb?

I've trimmed it down to a few relevant files (one per folder) along with their subordinate .rubocop.yml files.

does it occur with --only Style/Documentation?

Yes.

is it only with -a?

No, rubocop and rubocop -a fail in the same way.

does it occur only in VS Code or can it be reproduced just running in a Git Bash window?

Only in VS Code, and from my experience, only when:

  • using a workspace (ie more than one opened folder in the same window)
  • calling the integrated terminal
  • when Code asks you to select the folder, choosing it and not changing working directory

As soon as you change CWD, the error is gone.

You said rails-rspec, but meant rubocop-rspec, right?

Yes, my bad.

As for the reduced test case, you'll find it here -- https://github.com/amyspark/rubocop-vs-code-bug . You should be able to reproduce this by loading it to a VS Code workspace and running rubocop on the folder.

@jonas054
Copy link
Collaborator

jonas054 commented Jul 19, 2019

Thanks for the detailed description! Using that example project I was able to reprocude the problem in VS Code. I got the squiggly line under that class keyword in application.rb and the tooltip offense report from Style/Documentation.

But I have not been able (just as you hinted) to reproduce it in a shell. Not in Git Bash and not in CMD. This leads me to believe that the problem lies in vscode-ruby-rubocop and should be reported there.

Turning on Developer Tools in the Help menu of VS Code, I saw the following log message:

[Extension Host] rubocop.bat -a -f simple -c c:\Users\jonas\dev\7190\amyspark\rubocop-vs-code-bug\config\.rubocop.yml C:\Users\jonas\AppData\Local\Temp\rubocopRN1fb0\tmp.rb

If RuboCop is run like that, excludes are not going to work. The file name becomes something different when copying to a temporary file, and the exclude functionality depends on the file name.

Implementing a safe auto-correct from within an IDE may require some new feature in RuboCop. I don't have a solution ready, but I'm sure we can make it work. EDIT: Actually, it should be OK to auto-correct in the original location. I see that vscode-ruby-rubocop copies to a temporary file, but I'm not sure why.

I'm closing the issue for the reasons metioned above. Let me know if you don't agree.

@amyspark
Copy link
Author

amyspark commented Jul 19, 2019

Hey @jonas054 ,

Glad to see you could reproduce it! However,

But I have not been able (just as you hinted) to reproduce it in a shell. Not in Git Bash and not in CMD. This leads me to believe that the problem lies in vscode-ruby-rubocop and should be reported there.

I do not use any Ruby-related extensions in my VS Code installation. I only run Rubocop from within the integrated terminal, with terminal.integrated.shell.windows pointing to "d:\\Program Files\\Git\\bin\\bash.exe".

Git Bash itself works correctly, it's only in VS Code's integrated terminal that this bug appears.

@jonas054
Copy link
Collaborator

I think I finally managed to reproduce your problem as well.

I also noticed the following behavior:

jonas@LAPTOP-A32CA59G MINGW64 ~/dev/7190/amyspark/rubocop-vs-code-bug (master)
$ ruby -e 'puts Dir.pwd'
c:/Users/jonas/dev/7190/amyspark/rubocop-vs-code-bug

jonas@LAPTOP-A32CA59G MINGW64 ~/dev/7190/amyspark/rubocop-vs-code-bug (master)
$ cd .

jonas@LAPTOP-A32CA59G MINGW64 ~/dev/7190/amyspark/rubocop-vs-code-bug (master)
$ ruby -e 'puts Dir.pwd'
C:/Users/jonas/dev/7190/amyspark/rubocop-vs-code-bug

See how the cd . changes the drive letter to upper case? This is what trips up RuboCop, and I think the best solution is if we handle lower case drive letters as well as upper case. A change is needed in PathUtil#absolute?. I'll submit a PR.

@jonas054 jonas054 reopened this Jul 21, 2019
@amyspark
Copy link
Author

I did a bit more digging, and this seems to be a known issue when running shells inside VS Code: microsoft/vscode#9448, microsoft/vscode#45760, microsoft/vscode#70004 .

@koic koic closed this as completed in 23d3475 Jul 23, 2019
tas50 added a commit to chef/cookstyle that referenced this issue Oct 30, 2019
This resolves a large number of warnings and gives us a new cop for
automatically migrating the namespace of the cop disables in comments

* [#7407](rubocop/rubocop#7407): Make `Style/FormatStringToken` work inside hashes. ([@buehmann][])
* [#7389](rubocop/rubocop#7389): Fix an issue where passing a formatter might result in an error depending on what character it started with. ([@jfhinchcliffe][])
* [#7397](rubocop/rubocop#7397): Fix extra comments being added to the correction of `Style/SafeNavigation`. ([@rrosenblum][])
* [#7378](rubocop/rubocop#7378): Fix heredoc edge cases in `Layout/EmptyLineAfterGuardClause`. ([@gsamokovarov][])
* [#7404](rubocop/rubocop#7404): Fix a false negative for `Layout/IndentAssignment` when multiple assignment with line breaks on each line. ([@koic][])
* [#7256](rubocop/rubocop#7256): Fix an error of `Style/RedundantParentheses` on method calls where the first argument begins with a hash literal. ([@halfwhole][])
* [#7263](rubocop/rubocop#7263): Make `Layout/SpaceInsideArrayLiteralBrackets` properly handle tab-indented arrays. ([@buehmann][])
* [#7252](rubocop/rubocop#7252): Prevent infinite loops by making `Layout/SpaceInsideStringInterpolation` skip over interpolations that start or end with a line break. ([@buehmann][])
* [#7262](rubocop/rubocop#7262): `Lint/FormatParameterMismatch` did not recognize named format sequences like `%.2<name>f` where the name appears after some modifiers. ([@buehmann][])
* [#7253](rubocop/rubocop#7253): Fix an error for `Lint/NumberConversion` when `#to_i` called without a receiver. ([@koic][])
* [#7271](rubocop/rubocop#7271), [#6498](rubocop/rubocop#6498): Fix an interference between `Style/TrailingCommaIn*Literal` and `Layout/Multiline*BraceLayout` for arrays and hashes. ([@buehmann][])
* [#7241](rubocop/rubocop#7241): Make `Style/FrozenStringLiteralComment` match only true & false. ([@tejasbubane][])
* [#7290](rubocop/rubocop#7290): Handle inner conditional inside `else` in `Style/ConditionalAssignment`. ([@jonas054][])
* [#5788](rubocop/rubocop#5788): Allow block arguments on separate lines if line would be too long in `Layout/MultilineBlockLayout`. ([@jonas054][])
* [#7305](rubocop/rubocop#7305): Register `Style/BlockDelimiters` offense when block result is assigned to an attribute. ([@mvz][])
* [#4802](rubocop/rubocop#4802): Don't leave any `Lint/UnneededCopEnableDirective` offenses undetected/uncorrected. ([@jonas054][])
* [#7326](rubocop/rubocop#7326): Fix a false positive for `Style/AccessModifierDeclarations` when access modifier name is used for hash literal value. ([@koic][])
* [#3591](rubocop/rubocop#3591): Handle modifier `if`/`unless` correctly in `Lint/UselessAssignment`. ([@jonas054][])
* [#7161](rubocop/rubocop#7161): Fix `Style/SafeNavigation` cop for preserve comments inside if expression. ([@tejasbubane][])
* [#5212](rubocop/rubocop#5212): Avoid false positive for braces that are needed to preserve semantics in `Style/BracesAroundHashParameters`. ([@jonas054][])
* [#7353](rubocop/rubocop#7353): Fix a false positive for `Style/RedundantSelf` when receiver and multiple assigned lvalue have the same name. ([@koic][])
* [#7353](rubocop/rubocop#7353): Fix a false positive for `Style/RedundantSelf` when a self receiver is used as a method argument. ([@koic][])
* [#7358](rubocop/rubocop#7358): Fix an incorrect autocorrect for `Style/NestedModifier` when parentheses are required in method arguments. ([@koic][])
* [#7361](rubocop/rubocop#7361): Fix a false positive for `Style/TernaryParentheses` when only the closing parenthesis is used in the last line of condition. ([@koic][])
* [#7369](rubocop/rubocop#7369): Fix an infinite loop error for `Layout/IndentAssignment` with `Layout/IndentFirstArgument` when using multiple assignment. ([@koic][])
* [#7177](rubocop/rubocop#7177), [#7370](rubocop/rubocop#7370): When correcting alignment, do not insert spaces into string literals. ([@buehmann][])
* [#7367](rubocop/rubocop#7367): Fix an error for `Style/OrAssignment` cop when `then` branch body is empty. ([@koic][])
* [#7363](rubocop/rubocop#7363): Fix an incorrect autocorrect for `Layout/SpaceInsideBlockBraces` and `Style/BlockDelimiters` when using multiline empty braces. ([@koic][])
* [#7212](rubocop/rubocop#7212): Fix a false positive for `Layout/EmptyLinesAroundAccessModifier` and `UselessAccessModifier` when using method with the same name as access modifier around a method definition. ([@koic][])
* [#7217](rubocop/rubocop#7217): Make `Style/TrailingMethodEndStatement` work on more than the first `def`. ([@buehmann][])
* [#7190](rubocop/rubocop#7190): Support lower case drive letters on Windows. ([@jonas054][])
* Fix the auto-correction of `Lint/UnneededSplatExpansion` when the splat expansion of `Array.new` with a block is assigned to a variable. ([@rrosenblum][])
* [#5628](rubocop/rubocop#5628): Fix an error of `Layout/SpaceInsideStringInterpolation` on interpolations with multiple statements. ([@buehmann][])
* [#7128](rubocop/rubocop#7128): Make `Metrics/LineLength` aware of shebang. ([@koic][])
* [#6861](rubocop/rubocop#6861): Fix a false positive for `Layout/IndentationWidth` when using `EnforcedStyle: outdent` of `Layout/AccessModifierIndentation`. ([@koic][])
* [#7235](rubocop/rubocop#7235): Fix an error where `Style/ConditionalAssignment` would swallow a nested `if` condition. ([@buehmann][])
* [#7242](rubocop/rubocop#7242): Make `Style/ConstantVisibility` work on non-trivial class and module bodies. ([@buehmann][])
* [#5265](rubocop/rubocop#5265): Improved `Layout/ExtraSpacing` cop to handle nested consecutive assignments. ([@jfelchner][])
* [#7215](rubocop/rubocop#7215): Make it clear what's wrong in the message from `Style/GuardClause`. ([@jonas054][])
* [#7245](rubocop/rubocop#7245): Make cops detect string interpolations in more contexts: inside of backticks, regular expressions, and symbols. ([@buehmann][])
 [#7275](rubocop/rubocop#7275): Make `Style/VariableName` aware argument names when invoking a method. ([@koic][])
* [#3534](rubocop/rubocop#3534): Make `Style/IfUnlessModifier` report and auto-correct modifier lines that are too long. ([@jonas054][])
* [#7261](rubocop/rubocop#7261): `Style/FrozenStringLiteralComment` no longer inserts an empty line after the comment. This is left to `Layout/EmptyLineAfterMagicComment`. ([@buehmann][])
* [#7091](rubocop/rubocop#7091): `Style/FormatStringToken` now detects format sequences with flags and modifiers. ([@buehmann][])
* [#7319](rubocop/rubocop#7319): Rename `IgnoredMethodPatterns` option to `IgnoredPatterns` option for `Style/MethodCallWithArgsParentheses`. ([@koic][])
* [#7345](rubocop/rubocop#7345): Mark unsafe for `Style/YodaCondition`. ([@koic][])
* [#7170](rubocop/rubocop#7170): Fix a false positive for `Layout/RescueEnsureAlignment` when def line is preceded with `private_class_method`. ([@tatsuyafw][])
* [#7186](rubocop/rubocop#7186): Fix a false positive for `Style/MixinUsage` when using inside multiline block and `if` condition is after `include`. ([@koic][])
* [#7099](rubocop/rubocop#7099): Fix an error of `Layout/RescueEnsureAlignment` on assigned blocks. ([@tatsuyafw][])
* [#5088](rubocop/rubocop#5088): Fix an error of `Layout/MultilineMethodCallIndentation` on method chains inside an argument. ([@buehmann][])
* [#4719](rubocop/rubocop#4719): Make `Layout/Tab` detect tabs between string literals. ([@buehmann][])
* [#7203](rubocop/rubocop#7203): Fix an infinite loop error for `Layout/SpaceInsideBlockBraces` when `EnforcedStyle: no_space` with `SpaceBeforeBlockParameters: false` are set in multiline block. ([@koic][])
* [#6653](rubocop/rubocop#6653): Fix a bug where `Layout/IndentHeredoc` would remove empty lines when autocorrecting heredocs. ([@buehmann][])
* [#7188](rubocop/rubocop#7188): Include inspected file location in auto-correction error. ([@pocke][])

Signed-off-by: Tim Smith <tsmith@chef.io>
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

No branches or pull requests

2 participants