AtomLinter / linter-codeclimate

An Atom Linter plugin for the Code Climate CLI
http://github.com/codeclimate/codeclimate
MIT License
10 stars 5 forks source link

ignoring exclude_paths? #68

Open soderlind opened 7 years ago

soderlind commented 7 years ago
macos 10.12.4
codeclimate 0.63.3
linter-codeclimate 0.2.4

The exclude_paths rules are followed when running codeclimate analyze, but in linter-codeclimate (phpcodesniffer) they are not:

linter-codeclimate-exclude_paths

Below are my .codeclimate.yml and phpcs.xml files:

---
engines:
  duplication:
    enabled: true
    config:
      languages:
        - php
  eslint:
    enabled: true
  fixme:
    enabled: true
  phpcodesniffer:
    enabled: true
    config:
        standard: "phpcs.xml"
ratings:
  paths:
    - "**.js"
    - "**.php"
exclude_paths:
  - node_modules/**
  - languages/**
  - lib/cloudinary/**
  - lib/plugin-customizer/assets/twentysixteen/**
  - lib/Customizer/assets/js/boxshadow.js
<?xml version="1.0"?>
<ruleset name="Plugin Customizer">
    <description>The code standard for Plugin Customizer in WordPress.</description>

    <!-- ##### WordPress sniffs #####-->
    <rule ref="WordPress-Core">
        <exclude name="Generic.Files.LowercasedFilename" />
        <exclude name="WordPress.Files.FileName" />
        <exclude name="WordPress.Files.FileName.UnderscoresNotAllowed" />
    </rule>
</ruleset>
maxjacobson commented 7 years ago

Thanks for the report. I think I know what's going on here.

If you run codeclimate analyze foo.php, the CLI will print out issues even if foo.php is excluded. It assumes that if you are providing a path to a file, you want it to be analyzed, even if it is otherwise excluded. The atom package shells out the CLI and runs codeclimate analyze -f json foo.php.

I think this behavior is expected based on the implementation, but I could see how it's not how you'd expect it to behave. Will you outline what your expectations are? I could imagine adding a flag to the CLI such as --respect-excludes, which the package would use. Can you think of any cases where that wouldn't do the right thing?

soderlind commented 7 years ago

My expectation is that linter-codeclimate respects all settings in .codeclimate.yml

If --respect-excludes gives me this, it's "a" fix (but respecting settings in .codeclimate.yml would be better)

Arcanemagus commented 7 years ago

Most linters actually follow this current behavior: If you give an explicit path it is ran even if the configuration file says to ignore it. There are some that do ignore it anyway, sometimes printing a warning that it is being ignored. Those that follow the second pattern have an option to ignore the ignoring.

As far as implementing it here, the above option is probably just as good a solution as others.