no-unpublished-require/no-unpublished-bin false positives

[aarongoldenthal]

In published bin files in npm packages, there are false positive for the no-unpublished-require and no-unpublished-bin rules when a path is included in the package.json files array relative to ..

For example, with package.json:

  "bin": "./bin/foo.js",
  "exports": "./index.js",
  "files": [
    "index.js",
    "bin/",
    "lib/"
  ]

The following ./bin/foo.js does not show any no-unpublished-require errors:

#!/usr/bin/env node
'use strict';

const foo = require('..');
const bar= require('../lib/bar.js');

If the package.json is changed to:

  "bin": "./bin/foo.js",
  "exports": "./index.js",
  "files": [
    "./index.js",
    "bin/",
    "./lib/"
  ]

then ./bin/foo.js shows a no-unpublished-require error for both requires. This seems to only occur in bin files. For example, if ./lib/bar.js includes a require('../') it doe snot show a no-unpublished-require` error. The required files are, however, published.

Similarly, if the files entry bin/ is changed to ./bin/, then the entire ./bin/foo.js file shows a no-unpublished-bin error (it seems related).

There's an example at https://github.com/aarongoldenthal/eslint-n-require-false-positive.

[scagood]

This looks to be the same as #99

I will see if I can find some time this week to look

[scagood]

🤔 So, you can avoid this by simply changing the regex at https://github.com/eslint-community/eslint-plugin-n/blob/master/lib/util/get-npmignore.js#L15

From	To

---

This is a simple solution, but I think a better solution would be to use path.normalize:

Something like this:

const body = path
  .normalize(file.replace(/^!/u, ''))
  .replace(/\/+$/u, '')

🖼️ Screenshot

🏗️ 🩹 Git Patch

diff --git a/lib/util/get-npmignore.js b/lib/util/get-npmignore.js
index a818075..4483308 100644
--- a/lib/util/get-npmignore.js
+++ b/lib/util/get-npmignore.js
@@ -12,7 +12,6 @@ const exists = require("./exists")
 const getPackageJson = require("./get-package-json")
 
 const cache = new Cache()
-const SLASH_AT_BEGIN_AND_END = /^!?\/+|^!|\/+$/gu
 const PARENT_RELATIVE_PATH = /^\.\./u
 const NEVER_IGNORED =
     /^(?:readme\.[^.]*|(?:licen[cs]e|changes|changelog|history)(?:\.[^.]*)?)$/iu
@@ -90,7 +89,10 @@ function parseWhiteList(files) {
 
     for (const file of files) {
         if (typeof file === "string" && file) {
-            const body = file.replace(SLASH_AT_BEGIN_AND_END, "")
+            const body = path
+                .normalize(file.replace(/^!/u, ''))
+                .replace(/\/+$/u, '')
+
             if (file.startsWith("!")) {
                 igN.add(`${body}`)
                 igN.add(`${body}/**`)

This is because we can handle more things in the files array, (eg most of this silly) for no extra cost:

Input	Normalised
index.js	index.js
./index.js	index.js
././index.js	index.js
!index.js	index.js
!./index.js	index.js
!././index.js	index.js
forward/./index.js	forward/index.js
forward/../index.js	index.js
./forward/./index.js	forward/index.js
./forward/../index.js	index.js
!forward/./index.js	forward/index.js
!forward/../index.js	index.js
!./forward/./index.js	forward/index.js
!./forward/../index.js	index.js