From 5609abb0420554a955e1ca004eabf11ea4383657 Mon Sep 17 00:00:00 2001 From: Sebastian Good <2230835+scagood@users.noreply.github.com> Date: Wed, 7 Feb 2024 02:21:24 +0000 Subject: [PATCH] feat(shebang): Add options to ignore unpublished files (#172) * feat: Add shebangs to all ignored executable files * chore: Add names to shebang tests * fix: Ignore shebangs for all files not published * feat(shebang): Add "ignoreUnpublished" option * chore: Actually ignore test fixtures * chore: Remove import-maps module disable * feat(shebang): Add "additionalExecutables" option * docs(shebang): Add two new options to docs * chore(shebang): Only report the first line #85 --- docs/rules/shebang.md | 14 +- eslint.config.js | 5 +- lib/rules/shebang.js | 68 +++++++--- .../fixtures/shebang/unpublished/package.json | 7 + tests/lib/rules/shebang.js | 121 +++++++++++++++++- 5 files changed, 193 insertions(+), 22 deletions(-) create mode 100644 tests/fixtures/shebang/unpublished/package.json diff --git a/docs/rules/shebang.md b/docs/rules/shebang.md index 205e5d11..126daea5 100644 --- a/docs/rules/shebang.md +++ b/docs/rules/shebang.md @@ -61,7 +61,11 @@ console.log("hello"); ```json { - "n/shebang": ["error", {"convertPath": null}] + "n/shebang": ["error", { + "convertPath": null, + "ignoreUnpublished": false, + "additionalExecutables": [], + }] } ``` @@ -70,6 +74,14 @@ console.log("hello"); This can be configured in the rule options or as a shared setting [`settings.convertPath`](../shared-settings.md#convertpath). Please see the shared settings documentation for more information. +#### ignoreUnpublished + +Allow for files that are not published to npm to be ignored by this rule. + +#### additionalExecutables + +Mark files as executable that are not referenced by the package.json#bin property + ## 🔎 Implementation - [Rule source](../../lib/rules/shebang.js) diff --git a/eslint.config.js b/eslint.config.js index 3534ae88..6542c613 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -13,9 +13,6 @@ module.exports = [ { languageOptions: { globals: globals.mocha }, linterOptions: { reportUnusedDisableDirectives: true }, - settings: { - n: { allowModules: ["#eslint-rule-tester"] }, // the plugin does not support import-maps yet. - }, }, { ignores: [ @@ -23,7 +20,7 @@ module.exports = [ "coverage/", "docs/", "lib/converted-esm/", - "test/fixtures/", + "tests/fixtures/", ], }, js.configs.recommended, diff --git a/lib/rules/shebang.js b/lib/rules/shebang.js index 179f347c..c4f9d4ef 100644 --- a/lib/rules/shebang.js +++ b/lib/rules/shebang.js @@ -5,8 +5,11 @@ "use strict" const path = require("path") +const matcher = require("ignore") + const getConvertPath = require("../util/get-convert-path") const getPackageJson = require("../util/get-package-json") +const getNpmignore = require("../util/get-npmignore") const NODE_SHEBANG = "#!/usr/bin/env node\n" const SHEBANG_PATTERN = /^(#!.+?)?(\r)?\n/u @@ -66,6 +69,7 @@ function getShebangInfo(sourceCode) { } } +/** @type {import('eslint').Rule.RuleModule} */ module.exports = { meta: { docs: { @@ -79,8 +83,12 @@ module.exports = { { type: "object", properties: { - // convertPath: getConvertPath.schema, + ignoreUnpublished: { type: "boolean" }, + additionalExecutables: { + type: "array", + items: { type: "string" }, + }, }, additionalProperties: false, }, @@ -95,30 +103,60 @@ module.exports = { }, create(context) { const sourceCode = context.sourceCode ?? context.getSourceCode() // TODO: just use context.sourceCode when dropping eslint < v9 - let filePath = context.filename ?? context.getFilename() + const filePath = context.filename ?? context.getFilename() if (filePath === "") { return {} } - filePath = path.resolve(filePath) const p = getPackageJson(filePath) if (!p) { return {} } - const basedir = path.dirname(p.filePath) - filePath = path.join( - basedir, - getConvertPath(context)( - path.relative(basedir, filePath).replace(/\\/gu, "/") - ) + const packageDirectory = path.dirname(p.filePath) + + const originalAbsolutePath = path.resolve(filePath) + const originalRelativePath = path + .relative(packageDirectory, originalAbsolutePath) + .replace(/\\/gu, "/") + + const convertedRelativePath = + getConvertPath(context)(originalRelativePath) + const convertedAbsolutePath = path.resolve( + packageDirectory, + convertedRelativePath ) - const needsShebang = isBinFile(filePath, p.bin, basedir) + const { additionalExecutables = [] } = context.options?.[0] ?? {} + + const executable = matcher() + executable.add(additionalExecutables) + const isExecutable = executable.test(convertedRelativePath) + + if ( + (additionalExecutables.length === 0 || + isExecutable.ignored === false) && + context.options?.[0]?.ignoreUnpublished === true + ) { + const npmignore = getNpmignore(convertedAbsolutePath) + + if (npmignore.match(convertedRelativePath)) { + return {} + } + } + + const needsShebang = + isExecutable.ignored === true || + isBinFile(convertedAbsolutePath, p.bin, packageDirectory) const info = getShebangInfo(sourceCode) return { - Program(node) { + Program() { + const loc = { + start: { line: 1, column: 0 }, + end: { line: 1, column: sourceCode.lines.at(0).length }, + } + if ( needsShebang ? NODE_SHEBANG_PATTERN.test(info.shebang) @@ -128,7 +166,7 @@ module.exports = { // Checks BOM and \r. if (needsShebang && info.bom) { context.report({ - node, + loc, messageId: "unexpectedBOM", fix(fixer) { return fixer.removeRange([-1, 0]) @@ -137,7 +175,7 @@ module.exports = { } if (needsShebang && info.cr) { context.report({ - node, + loc, messageId: "expectedLF", fix(fixer) { const index = sourceCode.text.indexOf("\r") @@ -148,7 +186,7 @@ module.exports = { } else if (needsShebang) { // Shebang is lacking. context.report({ - node, + loc, messageId: "expectedHashbangNode", fix(fixer) { return fixer.replaceTextRange( @@ -160,7 +198,7 @@ module.exports = { } else { // Shebang is extra. context.report({ - node, + loc, messageId: "expectedHashbang", fix(fixer) { return fixer.removeRange([0, info.length]) diff --git a/tests/fixtures/shebang/unpublished/package.json b/tests/fixtures/shebang/unpublished/package.json new file mode 100644 index 00000000..05fe8a95 --- /dev/null +++ b/tests/fixtures/shebang/unpublished/package.json @@ -0,0 +1,7 @@ +{ + "name": "test", + "version": "0.0.0", + "files": [ + "./published.js" + ] +} diff --git a/tests/lib/rules/shebang.js b/tests/lib/rules/shebang.js index 47d9460c..f0e0f9c1 100644 --- a/tests/lib/rules/shebang.js +++ b/tests/lib/rules/shebang.js @@ -17,63 +17,82 @@ function fixture(name) { return path.resolve(__dirname, "../../fixtures/shebang", name) } +/** @type {import('eslint').RuleTester} */ const ruleTester = new RuleTester() ruleTester.run("shebang", rule, { valid: [ { + name: "string-bin/bin/test.js", filename: fixture("string-bin/bin/test.js"), code: "#!/usr/bin/env node\nhello();", }, { + name: "string-bin/lib/test.js", filename: fixture("string-bin/lib/test.js"), code: "hello();", }, { + name: "object-bin/bin/a.js", filename: fixture("object-bin/bin/a.js"), code: "#!/usr/bin/env node\nhello();", }, { + name: "object-bin/bin/b.js", filename: fixture("object-bin/bin/b.js"), code: "#!/usr/bin/env node\nhello();", }, { + name: "object-bin/bin/c.js", filename: fixture("object-bin/bin/c.js"), code: "hello();", }, { + name: "no-bin-field/lib/test.js", filename: fixture("no-bin-field/lib/test.js"), code: "hello();", }, - "#!/usr/bin/env node\nhello();", - "hello();", + { + name: " with shebang", + code: "#!/usr/bin/env node\nhello();", + }, + { + name: " without shebang", + code: "hello();", + }, // convertPath { + name: "convertPath - string-bin/src/bin/test.js", filename: fixture("string-bin/src/bin/test.js"), code: "#!/usr/bin/env node\nhello();", options: [{ convertPath: { "src/**": ["^src/(.+)$", "$1"] } }], }, { + name: "convertPath - string-bin/src/lib/test.js", filename: fixture("string-bin/src/lib/test.js"), code: "hello();", options: [{ convertPath: { "src/**": ["^src/(.+)$", "$1"] } }], }, { + name: "convertPath - object-bin/src/bin/a.js", filename: fixture("object-bin/src/bin/a.js"), code: "#!/usr/bin/env node\nhello();", options: [{ convertPath: { "src/**": ["^src/(.+)$", "$1"] } }], }, { + name: "convertPath - object-bin/src/bin/b.js", filename: fixture("object-bin/src/bin/b.js"), code: "#!/usr/bin/env node\nhello();", options: [{ convertPath: { "src/**": ["^src/(.+)$", "$1"] } }], }, { + name: "convertPath - object-bin/src/bin/c.js", filename: fixture("object-bin/src/bin/c.js"), code: "hello();", options: [{ convertPath: { "src/**": ["^src/(.+)$", "$1"] } }], }, { + name: "convertPath - no-bin-field/src/lib/test.js", filename: fixture("no-bin-field/src/lib/test.js"), code: "hello();", options: [{ convertPath: { "src/**": ["^src/(.+)$", "$1"] } }], @@ -81,88 +100,131 @@ ruleTester.run("shebang", rule, { // Should work fine if the filename is relative. { + name: "relative path - string-bin/bin/test.js", filename: "tests/fixtures/shebang/string-bin/bin/test.js", code: "#!/usr/bin/env node\nhello();", }, { + name: "relative path - string-bin/lib/test.js", filename: "tests/fixtures/shebang/string-bin/lib/test.js", code: "hello();", }, // BOM and \r\n { + name: "BOM without newline", filename: fixture("string-bin/lib/test.js"), code: "\uFEFFhello();", }, { + name: "BOM with newline", filename: fixture("string-bin/lib/test.js"), code: "\uFEFFhello();\n", }, { + name: "with windows newline", filename: fixture("string-bin/lib/test.js"), code: "hello();\r\n", }, { + name: "BOM with windows newline", filename: fixture("string-bin/lib/test.js"), code: "\uFEFFhello();\r\n", }, // blank lines on the top of files. { + name: "blank lines on the top of files.", filename: fixture("string-bin/lib/test.js"), code: "\n\n\nhello();", }, // https://github.com/mysticatea/eslint-plugin-node/issues/51 { + name: "Shebang with CLI flags", filename: fixture("string-bin/bin/test.js"), code: "#!/usr/bin/env node --harmony\nhello();", }, // use node resolution { + name: "use node resolution", filename: fixture("object-bin/bin/index.js"), code: "#!/usr/bin/env node\nhello();", }, + + // npm unpublished files are ignored + { + name: "published file cant have shebang", + filename: fixture("unpublished/published.js"), + code: "hello();", + options: [{ ignoreUnpublished: true }], + }, + { + name: "unpublished file can have shebang", + filename: fixture("unpublished/unpublished.js"), + code: "#!/usr/bin/env node\nhello();", + options: [{ ignoreUnpublished: true }], + }, + { + name: "unpublished file can have noshebang", + filename: fixture("unpublished/unpublished.js"), + code: "hello();", + options: [{ ignoreUnpublished: true }], + }, + + { + name: "file matching additionalExecutables", + filename: fixture("unpublished/something.test.js"), + code: "#!/usr/bin/env node\nhello();", + options: [{ additionalExecutables: ["*.test.js"] }], + }, ], invalid: [ { + name: "bin: string - match - no shebang", filename: fixture("string-bin/bin/test.js"), code: "hello();", output: "#!/usr/bin/env node\nhello();", errors: ['This file needs shebang "#!/usr/bin/env node".'], }, { + name: "bin: string - match - incorrect shebang", filename: fixture("string-bin/bin/test.js"), code: "#!/usr/bin/node\nhello();", output: "#!/usr/bin/env node\nhello();", errors: ['This file needs shebang "#!/usr/bin/env node".'], }, { + name: "bin: string - no match - with shebang", filename: fixture("string-bin/lib/test.js"), code: "#!/usr/bin/env node\nhello();", output: "hello();", errors: ["This file needs no shebang."], }, { + name: 'bin: {a: "./bin/a.js"} - match - no shebang', filename: fixture("object-bin/bin/a.js"), code: "hello();", output: "#!/usr/bin/env node\nhello();", errors: ['This file needs shebang "#!/usr/bin/env node".'], }, { + name: 'bin: {b: "./bin/b.js"} - match - no shebang', filename: fixture("object-bin/bin/b.js"), code: "#!/usr/bin/node\nhello();", output: "#!/usr/bin/env node\nhello();", errors: ['This file needs shebang "#!/usr/bin/env node".'], }, { + name: 'bin: {c: "./bin"} - no match - with shebang', filename: fixture("object-bin/bin/c.js"), code: "#!/usr/bin/env node\nhello();", output: "hello();", errors: ["This file needs no shebang."], }, { + name: "bin: undefined - no match - with shebang", filename: fixture("no-bin-field/lib/test.js"), code: "#!/usr/bin/env node\nhello();", output: "hello();", @@ -171,6 +233,7 @@ ruleTester.run("shebang", rule, { // convertPath { + name: "convertPath in options", filename: fixture("string-bin/src/bin/test.js"), code: "hello();", output: "#!/usr/bin/env node\nhello();", @@ -178,6 +241,7 @@ ruleTester.run("shebang", rule, { errors: ['This file needs shebang "#!/usr/bin/env node".'], }, { + name: "convertPath in settings", filename: fixture("string-bin/src/bin/test.js"), code: "hello();", output: "#!/usr/bin/env node\nhello();", @@ -187,6 +251,7 @@ ruleTester.run("shebang", rule, { }, }, { + name: "converted path - string-bin/src/bin/test.js", filename: fixture("string-bin/src/bin/test.js"), code: "#!/usr/bin/node\nhello();", output: "#!/usr/bin/env node\nhello();", @@ -194,6 +259,7 @@ ruleTester.run("shebang", rule, { errors: ['This file needs shebang "#!/usr/bin/env node".'], }, { + name: "converted path - string-bin/src/lib/test.js", filename: fixture("string-bin/src/lib/test.js"), code: "#!/usr/bin/env node\nhello();", output: "hello();", @@ -201,6 +267,7 @@ ruleTester.run("shebang", rule, { errors: ["This file needs no shebang."], }, { + name: "converted path - object-bin/src/bin/a.js", filename: fixture("object-bin/src/bin/a.js"), code: "hello();", output: "#!/usr/bin/env node\nhello();", @@ -208,6 +275,7 @@ ruleTester.run("shebang", rule, { errors: ['This file needs shebang "#!/usr/bin/env node".'], }, { + name: "converted path - object-bin/src/bin/b.js", filename: fixture("object-bin/src/bin/b.js"), code: "#!/usr/bin/node\nhello();", output: "#!/usr/bin/env node\nhello();", @@ -215,6 +283,7 @@ ruleTester.run("shebang", rule, { errors: ['This file needs shebang "#!/usr/bin/env node".'], }, { + name: "converted path - object-bin/src/bin/c.js", filename: fixture("object-bin/src/bin/c.js"), code: "#!/usr/bin/env node\nhello();", output: "hello();", @@ -222,6 +291,7 @@ ruleTester.run("shebang", rule, { errors: ["This file needs no shebang."], }, { + name: "converted path - no-bin-field/src/lib/test.js", filename: fixture("no-bin-field/src/lib/test.js"), code: "#!/usr/bin/env node\nhello();", output: "hello();", @@ -231,12 +301,14 @@ ruleTester.run("shebang", rule, { // Should work fine if the filename is relative. { + name: "relative path - string-bin/bin/test.js", filename: "tests/fixtures/shebang/string-bin/bin/test.js", code: "hello();", output: "#!/usr/bin/env node\nhello();", errors: ['This file needs shebang "#!/usr/bin/env node".'], }, { + name: "relative path - string-bin/lib/test.js", filename: "tests/fixtures/shebang/string-bin/lib/test.js", code: "#!/usr/bin/env node\nhello();", output: "hello();", @@ -245,6 +317,7 @@ ruleTester.run("shebang", rule, { // header comments { + name: "header comments", filename: fixture("string-bin/bin/test.js"), code: "/* header */\nhello();", output: "#!/usr/bin/env node\n/* header */\nhello();", @@ -306,6 +379,7 @@ ruleTester.run("shebang", rule, { // https://github.com/mysticatea/eslint-plugin-node/issues/51 { + name: "Shebang with CLI flags", filename: fixture("string-bin/lib/test.js"), code: "#!/usr/bin/env node --harmony\nhello();", output: "hello();", @@ -314,10 +388,53 @@ ruleTester.run("shebang", rule, { // use node resolution { + name: "use node resolution", filename: fixture("object-bin/bin/index.js"), code: "hello();", output: "#!/usr/bin/env node\nhello();", errors: ['This file needs shebang "#!/usr/bin/env node".'], }, + + // npm unpublished files are ignored + { + name: "unpublished file should not have shebang", + filename: fixture("unpublished/unpublished.js"), + code: "#!/usr/bin/env node\nhello();", + output: "hello();", + errors: ["This file needs no shebang."], + }, + { + name: "published file should have shebang", + filename: fixture("unpublished/published.js"), + code: "#!/usr/bin/env node\nhello();", + output: "hello();", + errors: ["This file needs no shebang."], + }, + + { + name: "unpublished file shebang ignored", + filename: fixture("unpublished/published.js"), + code: "#!/usr/bin/env node\nhello();", + options: [{ ignoreUnpublished: true }], + output: "hello();", + errors: ["This file needs no shebang."], + }, + + { + name: "executable in additionalExecutables without shebang", + filename: fixture("unpublished/something.test.js"), + code: "hello();", + options: [{ additionalExecutables: ["*.test.js"] }], + output: "#!/usr/bin/env node\nhello();", + errors: ['This file needs shebang "#!/usr/bin/env node".'], + }, + { + name: "file not in additionalExecutables with shebang", + filename: fixture("unpublished/not-a-test.js"), + code: "#!/usr/bin/env node\nhello();", + options: [{ additionalExecutables: ["*.test.js"] }], + output: "hello();", + errors: ["This file needs no shebang."], + }, ], })