fluid-project / fluid-lint-all

Consolidated linting logic free from any particular build technology
BSD 3-Clause "New" or "Revised" License
0 stars 5 forks source link

Supplied excludes for lintspaces.newlines in .fluidlintallrc.json overwrite some default excludes #21

Closed greatislander closed 3 years ago

greatislander commented 3 years ago

Describe the bug

For this project, a number of Markdown files are managed via Netlify CMS which means we can't enforce lintspaces.newlines or markdownlint checks on them. To address this, I added a block to .fluidlintallrc.json file as shown below:

    "lintspaces": {
        "newlines": {
            "excludes": [
                "./src/activities/*.md",
                "./src/events/*.md",
                "./src/lesson-plans/*.md",
                "./src/organizations/*.md",
                "./src/products/*.md",
                "./src/projects/*.md",
                "./src/resources/*.md",
                "./CHANGELOG.md"
            ]
        }
    },

However, when running the lint script, I noticed that I was getting lintspaces.newlines errors for a number of image files which should be excluded by the default excludes for this check (#6). Running the lint script with --showMergedConfig revealed that the merging of excludes appears to be overwriting some default excludes with those supplied by the .fluidlintallrc.json file:

  "lintspaces": {
    "newlines": {
      "excludes": [
        "./src/activities/*.md",
        "./src/events/*.md",
        "./src/lesson-plans/*.md",
        "./src/organizations/*.md",
        "./src/products/*.md",
        "./src/projects/*.md",
        "./src/resources/*.md",
        "./CHANGELOG.md",
        "*.webm",
        "*.webp"
      ],
      "enabled": true,
      "includes": [
        "./src/**/*",
        "./tests/**/*",
        "./*"
      ],
      "options": {
        "newline": true
      }
    },

It appears that the first eight lines from the default excludes array in lint-all.js are being replaced with my configuration, leaving only .webm and .webp: https://github.com/the-t-in-rtf/fluid-lint-all/blob/3cb114599f583843921299d18e37e6f4eceb950a/src/js/lint-all.js#L159-L167

To reproduce

Steps to reproduce the behavior:

  1. Create a .fluidlintallrc.json file with these contents.
  2. Run npx fluid-lint-all --showMergedConfig (or npm run lint -- --showMergedConfig if an npm lint script is present in the test project).
  3. Look for the merged configuration for lintspaces.newlines.
  4. See that many image types have been replaced with the supplied configuration.

Expected behavior

The default excludes should be present in the merged configuration.

Screenshots

Not applicable.

Technical details

Desktop

Additional context or notes

Not applicable.

the-t-in-rtf commented 3 years ago

This is a byproduct of using fluid's array merging. Our two options are either to migrate to using maps, or to explicitly avoid merging these by adding a merge policy. I'm leaning towards the latter, but will chat with @amb26 for a bit.

the-t-in-rtf commented 3 years ago

In rereading I see that you want to add your excludes to the existing ones. We need to discuss how someone could "opt out" of a default exclude in that scenario.

greatislander commented 3 years ago

@the-t-in-rtf Yes, I think the issue here is that the default excludes for lintspaces.newlines is not something most people, if anyone will want to opt out of— it should always be additive for that check specifically (because the default excludes prevent the check from trying to validate files which don't have newlines). But I take your point that there's a larger question of how to handle situations where people want an array of default excludes to be replaced instead of merged.

Maybe something like an options key for a check like this:

"stylelint": {
  "excludeStrategy": "merge" /* Alternatively: replace */
  "excludes": [],
}

The default behaviour would be merge, but using replace would replace the default excludes with user-supplied ones.

the-t-in-rtf commented 3 years ago

I came up with a new strategy to allow both adding and removing ("negating") individual entries. See #33 for details.