Negative .gitignore rules cause xo to pass everything

[JonnyBurger]

There is a regression in version 0.17, which in the worst case leads to xo ignoring all files and returning exit code 0, even though there are issues in the code. For instance, xo@0.17 will pass all tests in a react-native project no matter what because the .gitignore file has entries that start with !.

Test case

Here is a simple example project to demonstrate the issue:

---

package.json

{
  "name": "xo-bug-demo",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "xo"
  },
  "author": "Jonny <jonathanburger11@gmail.com> (http://jonny.io)",
  "license": "ISC",
  "devDependencies": {
    "xo": "^0.17.0"
  }
}

---

index.js

console.log('No semicolon – xo should fail')

---

.gitignore

some_folder
!some_folder/but_not_this_file

---

## Expected result

❯ xo

  index.js:1:45
  ✖  1:45  Missing semicolon.  semi

  1 error

Actual result

❯ xo

xo returns no error even though the semicolon is missing.
Running with xo@0.16 will yield the correct result.

Cause

Starting in 0.17, files in .gitignore are also ignored in xo. But there is a difference in how git and globby parse the ignore list.
In .gitignore, !a/b.js means: "Do not ignore this file".
In globby and glob, passing !a/b.js to the ignore option means: "Do ignore all files except this one"

[sindresorhus]

I wonder if we could simply filter out patterns starting with !, strip the ! and pass them as positive matches and the rest to the ignore option. Thoughts?

// @JonnyBurger @schnittstabil

[JonnyBurger]

@sindresorhus I think that will not work because negative matches override positive matches. So, let's say the .gitignore is

a
!a/b.js

, if we strip the ! and make it a positive match, then we pass it to globby:

globby(['**/*', 'a/b.js'], {ignore: ['a']})

then a/b.js gets ignored. We could achieve the same result by just filtering out the patterns starting with ! and do nothing with them.

That is why I was initially suggesting to change globby, but I have another idea. We could also do the matching in two steps:

const flatten = require('lodash.flatten')
Promise.all([
   globby(['**/*'], {ignore: ['a']}), // don't do '!a/b.js' rule just yet...
   globby(['a/b.js']) // but use it in second call.
])
.then(matches => flatten(matches))

[Qix-]

FWIW this has been a problem with globby for a while now. I was writing a build script once and ran into this exact problem.

[schnittstabil]

@Qix- Sorry, I can't follow you. Did you already solved this problem within your build script? If you did, would you mind sharing your thoughts how this issue should be solved?

[Qix-]

Manually parsing the beginning using something like ^(?:!!)*(!)? and seeing if the first matching group is not null, adding it to a list if it wasn't null (and ignoring it for the time being), run all of the globbing patterns that weren't negatives, and then apply the negatives to the list of found files to filter them out.

The inverse could be done here.

[schnittstabil]

Disclaimer: This is just an idea, I must admit, I do not have enough time to thoroughly figure out the impact of this atm.

As far as I can see, we do not need to use the ignore option of globby at all, if we sort the patterns:

const negate = patterns => patterns.map(p => p.startsWith('!') ? p.substr(1) : '!' + p);

const parseGitignore = patterns => negate(patterns).sort(p => p.startsWith('!') ? 1 : -1);

parseGitignore(['some_folder', '!some_folder/but_not_this_file'])
// => [ 'some_folder/but_not_this_file', '!some_folder' ]