Shopify / erb_lint

Lint your ERB or HTML files
MIT License
675 stars 122 forks source link

Bug: the exclusion pattern `**/*` does not work as expected #265

Open pil0u opened 2 years ago

pil0u commented 2 years ago

I have a project structure that looks like:

app
 |_ components
     |_ folder
         |_ some_file.html.erb
     |_ some_other_file.html.erb

I want to prevent a linter from analysing any file in the app/components folder. What I would do is:

HardCodedString:
  exclude:
    - '**/app/components/**/*'

The app/components/folder/some_file.html.erb is properly excluded, but the app/components/some_other_file.html.erb is not.

Is this expected?

rafaelfranca commented 2 years ago

This seems like a bug to me. How Rubocop works for those files?

pil0u commented 2 years ago

@rafaelfranca As an example, I have a rule in RuboCop that looks like

Lint/MissingSuper:
  Exclude:
    - "**/app/components/**/*"

and all my files in the app/components folder, nested in sub-folders or not, are properly excluded.

I would expect the same behaviour to work with erb-lint:

HardCodedString:
  exclude:
    - '**/app/components/**/*'

but my "non-nested" files are not properly excluded with that setup. My current workaround is:

HardCodedString:
  exclude:
    - '**/app/components/**/*'
    - '**/app/components/*'

which is very cheap, but feels a little off.

xiaohui-zhangxh commented 1 year ago

This bug relates https://github.com/Shopify/erb-lint/blob/9e53328bc5df06ebaa0d2a56706641b6d349c08f/lib/erb_lint/cli.rb#L258

Per File.fnmatch documentation : https://apidock.com/ruby/v2_6_3/File/fnmatch%3F/class

pattern = '**/app/components/**/*'
File.fnmatch(pattern, '/a/app/components/foo') #=> false
File.fnmatch(pattern, '/a/app/components/foo/bar') #=> true

File.fnmatch(pattern, '/a/app/components/foo', File::FNM_PATHNAME) #=> true
File.fnmatch(pattern, '/a/app/components/foo/bar', File::FNM_PATHNAME) #=> true

So, if we don't change the code, the correct exclude pattern should be:

exclude:
- '*/app/components/*'
pattern = '*/app/components/*'
File.fnmatch(pattern, '/a/app/components/foo') #=> false
File.fnmatch(pattern, '/a/app/components/foo/bar') #=> true