From 430ba921243e0b9ebcbf4043bf6d63de8f085a35 Mon Sep 17 00:00:00 2001 From: jbsingh Date: Mon, 20 Nov 2017 18:22:59 -0600 Subject: [PATCH 1/5] update importBlacklistRule to use regular expressions --- src/rules/importBlacklistRule.ts | 33 +++++++++++++++++------- test/rules/import-blacklist/test.ts.lint | 11 +++++--- test/rules/import-blacklist/tslint.json | 2 +- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/rules/importBlacklistRule.ts b/src/rules/importBlacklistRule.ts index c8a3dcb95a0..d5884e0cc6d 100644 --- a/src/rules/importBlacklistRule.ts +++ b/src/rules/importBlacklistRule.ts @@ -24,12 +24,19 @@ export class Rule extends Lint.Rules.AbstractRule { public static metadata: Lint.IRuleMetadata = { ruleName: "import-blacklist", description: Lint.Utils.dedent` - Disallows importing the specified modules directly via \`import\` and \`require\`. - Instead only sub modules may be imported from that module.`, + Disallows importing the specified modules directly via \`import\` and \`require\`.`, rationale: Lint.Utils.dedent` - Some libraries allow importing their submodules instead of the entire module. - This is good practise as it avoids loading unused modules.`, - optionsDescription: "A list of blacklisted modules.", + Some projects may wish to exclude imports matching certain patterns. + + Examples: + ^loadsh$ Blacklist any import of the entire lodash module. + Since lodash allows importing submodules, a better practice + would be to import the desired submodule instead. + + .*\\.temp$ Blacklist any imports ending in .temp`, + optionsDescription: Lint.Utils.dedent` + A list of blacklist pattern strings to be compiled to regular expressions. + The corresponding regular expressions will be tested against the imports.`, options: { type: "array", items: { @@ -37,12 +44,12 @@ export class Rule extends Lint.Rules.AbstractRule { }, minLength: 1, }, - optionExamples: [true, [true, "rxjs", "lodash"]], + optionExamples: [true, [true, "^rxjs$", "^lodash$", ".*\\.temp$"]], type: "functionality", typescriptOnly: false, }; - public static FAILURE_STRING = "This import is blacklisted, import a submodule instead"; + public static FAILURE_STRING = "This import is blacklisted by "; public isEnabled(): boolean { return super.isEnabled() && this.ruleArguments.length > 0; @@ -54,9 +61,17 @@ export class Rule extends Lint.Rules.AbstractRule { } function walk(ctx: Lint.WalkContext) { + const regexOptions = []; + for (const option of ctx.options) { + if (typeof option === "string") { + regexOptions.push(RegExp(option)); + } + } for (const name of findImports(ctx.sourceFile, ImportKind.All)) { - if (ctx.options.indexOf(name.text) !== -1) { - ctx.addFailure(name.getStart(ctx.sourceFile) + 1, name.end - 1, Rule.FAILURE_STRING); + for (const regex of regexOptions) { + if (regex.test(name.text)) { + ctx.addFailure(name.getStart(ctx.sourceFile) + 1, name.end - 1, Rule.FAILURE_STRING + regex.toString()); + } } } } diff --git a/test/rules/import-blacklist/test.ts.lint b/test/rules/import-blacklist/test.ts.lint index 14887486fd6..3aa7dbd3d57 100644 --- a/test/rules/import-blacklist/test.ts.lint +++ b/test/rules/import-blacklist/test.ts.lint @@ -3,7 +3,7 @@ import { Observable } from 'rxjs'; import { Observable } from 'rxjs/Observable'; import forOwn = require('lodash'); - ~~~~~~ [0] + ~~~~~~ [1] import forOwn = require('lodash/forOwn'); // non-static imports cannot be checked @@ -13,6 +13,11 @@ import forOwn = require(lodash); import * as notBlacklisted from "not-blacklisted"; export * from 'lodash'; - ~~~~~~ [0] + ~~~~~~ [1] -[0]: This import is blacklisted, import a submodule instead +import {myFunc} from './my.temp'; + ~~~~~~~~~ [2] + +[0]: This import is blacklisted by /^rxjs$/ +[1]: This import is blacklisted by /^lodash$/ +[2]: This import is blacklisted by /.*\.temp$/ \ No newline at end of file diff --git a/test/rules/import-blacklist/tslint.json b/test/rules/import-blacklist/tslint.json index 7abdb4f44a8..b8290666f6a 100644 --- a/test/rules/import-blacklist/tslint.json +++ b/test/rules/import-blacklist/tslint.json @@ -1,5 +1,5 @@ { "rules": { - "import-blacklist": [true, "lodash", "rxjs"] + "import-blacklist": [true, "^lodash$", "^rxjs$", ".*\\.temp$"] } } From 6a8d3f5b32d99647a8e04ac1fe4bb763a6390913 Mon Sep 17 00:00:00 2001 From: jbsingh Date: Thu, 30 Nov 2017 08:21:22 -0600 Subject: [PATCH 2/5] fix doc typo --- src/rules/importBlacklistRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/importBlacklistRule.ts b/src/rules/importBlacklistRule.ts index d5884e0cc6d..c5d8b0b718c 100644 --- a/src/rules/importBlacklistRule.ts +++ b/src/rules/importBlacklistRule.ts @@ -29,7 +29,7 @@ export class Rule extends Lint.Rules.AbstractRule { Some projects may wish to exclude imports matching certain patterns. Examples: - ^loadsh$ Blacklist any import of the entire lodash module. + ^lodash$ Blacklist any import of the entire lodash module. Since lodash allows importing submodules, a better practice would be to import the desired submodule instead. From 644d4ecb2333ceecb7ef55e5abef5090a7b3000e Mon Sep 17 00:00:00 2001 From: jbsingh Date: Thu, 11 Jan 2018 00:30:38 -0600 Subject: [PATCH 3/5] update to a backwards compatible solution --- src/rules/importBlacklistRule.ts | 74 ++++++++++++++++++------ test/rules/import-blacklist/test.ts.lint | 15 +++-- test/rules/import-blacklist/tslint.json | 2 +- 3 files changed, 66 insertions(+), 25 deletions(-) diff --git a/src/rules/importBlacklistRule.ts b/src/rules/importBlacklistRule.ts index c5d8b0b718c..5f363c237a6 100644 --- a/src/rules/importBlacklistRule.ts +++ b/src/rules/importBlacklistRule.ts @@ -26,30 +26,59 @@ export class Rule extends Lint.Rules.AbstractRule { description: Lint.Utils.dedent` Disallows importing the specified modules directly via \`import\` and \`require\`.`, rationale: Lint.Utils.dedent` - Some projects may wish to exclude imports matching certain patterns. - - Examples: - ^lodash$ Blacklist any import of the entire lodash module. - Since lodash allows importing submodules, a better practice - would be to import the desired submodule instead. - - .*\\.temp$ Blacklist any imports ending in .temp`, + Some libraries allow importing their submodules instead of the entire module. + This is good practise as it avoids loading unused modules. + Some projects may simply wish to exclude imports matching certain patterns.`, optionsDescription: Lint.Utils.dedent` - A list of blacklist pattern strings to be compiled to regular expressions. - The corresponding regular expressions will be tested against the imports.`, + A list of blacklisted modules and/or regular expression patterns.`, options: { type: "array", items: { - type: "string", + anyOf: [ + { + type: "string", + }, + { + type: "array", + items: { + type: "string", + }, + minLength: 1, + }, + ], }, minLength: 1, }, - optionExamples: [true, [true, "^rxjs$", "^lodash$", ".*\\.temp$"]], + optionExamples: [ + true, + [ + true, + "rxjs", + "lodash", + ], + [ + true, + [ + ".*\\.temp$", + ".*\\.tmp$", + ], + ], + [ + true, + "rxjs", + "lodash", + [ + ".*\\.temp$", + ".*\\.tmp$", + ], + ], + ], type: "functionality", typescriptOnly: false, }; - public static FAILURE_STRING = "This import is blacklisted by "; + public static FAILURE_STRING_MODULE = "This import is blacklisted, import a submodule instead"; + public static FAILURE_STRING_REGEX = "This import is blacklisted by "; public isEnabled(): boolean { return super.isEnabled() && this.ruleArguments.length > 0; @@ -60,17 +89,26 @@ export class Rule extends Lint.Rules.AbstractRule { } } -function walk(ctx: Lint.WalkContext) { +function walk(ctx: Lint.WalkContext) { + const moduleOptions = []; const regexOptions = []; for (const option of ctx.options) { if (typeof option === "string") { - regexOptions.push(RegExp(option)); + moduleOptions.push(option); + } else if (Array.isArray(option)) { + for (const pattern of option) { + regexOptions.push(RegExp(pattern)); + } } } for (const name of findImports(ctx.sourceFile, ImportKind.All)) { - for (const regex of regexOptions) { - if (regex.test(name.text)) { - ctx.addFailure(name.getStart(ctx.sourceFile) + 1, name.end - 1, Rule.FAILURE_STRING + regex.toString()); + if (moduleOptions.indexOf(name.text) !== -1) { + ctx.addFailure(name.getStart(ctx.sourceFile) + 1, name.end - 1, Rule.FAILURE_STRING_MODULE); + } else { + for (const regex of regexOptions) { + if (regex.test(name.text)) { + ctx.addFailure(name.getStart(ctx.sourceFile) + 1, name.end - 1, Rule.FAILURE_STRING_REGEX + regex.toString()); + } } } } diff --git a/test/rules/import-blacklist/test.ts.lint b/test/rules/import-blacklist/test.ts.lint index 3aa7dbd3d57..febc9506714 100644 --- a/test/rules/import-blacklist/test.ts.lint +++ b/test/rules/import-blacklist/test.ts.lint @@ -3,7 +3,7 @@ import { Observable } from 'rxjs'; import { Observable } from 'rxjs/Observable'; import forOwn = require('lodash'); - ~~~~~~ [1] + ~~~~~~ [0] import forOwn = require('lodash/forOwn'); // non-static imports cannot be checked @@ -13,11 +13,14 @@ import forOwn = require(lodash); import * as notBlacklisted from "not-blacklisted"; export * from 'lodash'; - ~~~~~~ [1] + ~~~~~~ [0] import {myFunc} from './my.temp'; - ~~~~~~~~~ [2] + ~~~~~~~~~ [1] -[0]: This import is blacklisted by /^rxjs$/ -[1]: This import is blacklisted by /^lodash$/ -[2]: This import is blacklisted by /.*\.temp$/ \ No newline at end of file +import {myFunc} from './my.tmp'; + ~~~~~~~~ [2] + +[0]: This import is blacklisted, import a submodule instead +[1]: This import is blacklisted by /.*\.temp$/ +[2]: This import is blacklisted by /.*\.tmp$/ \ No newline at end of file diff --git a/test/rules/import-blacklist/tslint.json b/test/rules/import-blacklist/tslint.json index b8290666f6a..b8e5fd72cab 100644 --- a/test/rules/import-blacklist/tslint.json +++ b/test/rules/import-blacklist/tslint.json @@ -1,5 +1,5 @@ { "rules": { - "import-blacklist": [true, "^lodash$", "^rxjs$", ".*\\.temp$"] + "import-blacklist": [true, "lodash", "rxjs", [".*\\.temp$", ".*\\.tmp$"]] } } From 90c33713224a184ba512d71b51abadc9f2bb7910 Mon Sep 17 00:00:00 2001 From: jbsingh Date: Mon, 17 Dec 2018 23:51:17 -0600 Subject: [PATCH 4/5] adjusted previous importBlacklistRule updates against latest code --- src/rules/importBlacklistRule.ts | 39 +++++++++++++++++++++--- test/rules/import-blacklist/test.ts.lint | 8 +++++ test/rules/import-blacklist/tslint.json | 2 +- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/rules/importBlacklistRule.ts b/src/rules/importBlacklistRule.ts index e4cee9a4ced..313c2a977f3 100644 --- a/src/rules/importBlacklistRule.ts +++ b/src/rules/importBlacklistRule.ts @@ -31,14 +31,16 @@ export class Rule extends Lint.Rules.AbstractRule { ruleName: "import-blacklist", description: Lint.Utils.dedent` Disallows importing the specified modules via \`import\` and \`require\`, - or importing specific named exports of the specified modules.`, + or importing specific named exports of the specified modules, + or using imports matching specified regular expression patterns.`, rationale: Lint.Utils.dedent` For some libraries, importing the library directly can cause unused submodules to be loaded, so you may want to block these imports and require that users directly import only the submodules they need. In other cases, you may simply want to ban an import because using it is undesirable or unsafe.`, - optionsDescription: "A list of blacklisted modules or named imports.", + optionsDescription: + "A list of blacklisted modules, named imports, or regular expression patterns.", options: { type: "array", items: { @@ -58,6 +60,13 @@ export class Rule extends Lint.Rules.AbstractRule { }, }, }, + { + type: "array", + items: { + type: "string", + }, + minLength: 1, + }, ], }, }, @@ -65,6 +74,7 @@ export class Rule extends Lint.Rules.AbstractRule { true, [true, "rxjs", "lodash"], [true, "lodash", { lodash: ["pull", "pullAll"] }], + [true, "rxjs", { lodash: ["pull", "pullAll"] }, [".*\\.temp$", ".*\\.tmp$"]], ], type: "functionality", typescriptOnly: false, @@ -84,6 +94,8 @@ export class Rule extends Lint.Rules.AbstractRule { : `The export "${importName}" is blacklisted.`; } + public static FAILURE_STRING_REGEX = "This import is blacklisted by "; + public isEnabled(): boolean { return super.isEnabled() && this.ruleArguments.length > 0; } @@ -93,7 +105,7 @@ export class Rule extends Lint.Rules.AbstractRule { } } -type Options = Array; +type Options = Array>; function walk(ctx: Lint.WalkContext) { interface BannedImports { @@ -107,7 +119,7 @@ function walk(ctx: Lint.WalkContext) { (acc, it) => { if (typeof it === "string") { acc[it] = true; - } else { + } else if (!Array.isArray(it)) { Object.keys(it).forEach(moduleName => { if (acc[moduleName] instanceof Set) { it[moduleName].forEach(bannedImport => { @@ -123,6 +135,15 @@ function walk(ctx: Lint.WalkContext) { Object.create(null) as BannedImports, ); + const regexOptions = []; + for (const option of ctx.options) { + if (Array.isArray(option)) { + for (const pattern of option) { + regexOptions.push(RegExp(pattern)); + } + } + } + for (const name of findImports(ctx.sourceFile, ImportKind.All)) { // TODO #3963: Resolve/normalize relative file imports to a canonical path? const importedModule = name.text; @@ -233,5 +254,15 @@ function walk(ctx: Lint.WalkContext) { ); } } + + for (const regex of regexOptions) { + if (regex.test(name.text)) { + ctx.addFailure( + name.getStart(ctx.sourceFile) + 1, + name.end - 1, + Rule.FAILURE_STRING_REGEX + regex.toString(), + ); + } + } } } diff --git a/test/rules/import-blacklist/test.ts.lint b/test/rules/import-blacklist/test.ts.lint index c66225fdc76..0c499ed2f39 100644 --- a/test/rules/import-blacklist/test.ts.lint +++ b/test/rules/import-blacklist/test.ts.lint @@ -40,8 +40,16 @@ import { pull as notPull } from "lodash"; import test from "dummy"; ~~~~ [3] +import {myFunc} from './my.temp'; + ~~~~~~~~~ [4] + +import {myFunc} from './my.tmp'; + ~~~~~~~~ [5] + [0]: Importing this module is blacklisted. Try importing a submodule instead. [1]: Some named exports from this module are blacklisted for importing (or re-exporting). Import/re-export only the specific values you want, instead of the whole module. [2]: The export "pull" is blacklisted. [3]: Importing (or re-exporting) the default export is blacklisted. +[4]: This import is blacklisted by /.*\.temp$/ +[5]: This import is blacklisted by /.*\.tmp$/ diff --git a/test/rules/import-blacklist/tslint.json b/test/rules/import-blacklist/tslint.json index 3c543ffa93f..5ac04a3762d 100644 --- a/test/rules/import-blacklist/tslint.json +++ b/test/rules/import-blacklist/tslint.json @@ -1,5 +1,5 @@ { "rules": { - "import-blacklist": [true, "rxjs", { "lodash": ["pullAll", "pull"] }, { "dummy": ["default"] }] + "import-blacklist": [true, "rxjs", { "lodash": ["pullAll", "pull"] }, { "dummy": ["default"] }, [".*\\.temp$", ".*\\.tmp$"]] } } From a36bba0ebe27c2986b7a7488b78855fc51001114 Mon Sep 17 00:00:00 2001 From: jbsingh Date: Tue, 18 Dec 2018 00:02:49 -0600 Subject: [PATCH 5/5] fixed lint errors --- src/rules/importBlacklistRule.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rules/importBlacklistRule.ts b/src/rules/importBlacklistRule.ts index 313c2a977f3..a4c55e3ef3b 100644 --- a/src/rules/importBlacklistRule.ts +++ b/src/rules/importBlacklistRule.ts @@ -88,14 +88,14 @@ export class Rule extends Lint.Rules.AbstractRule { "(or re-exporting). Import/re-export only the specific values you want, " + "instead of the whole module."; + public static FAILURE_STRING_REGEX = "This import is blacklisted by "; + public static MAKE_NAMED_IMPORT_FAILURE_STRING(importName: string) { return importName === "default" ? "Importing (or re-exporting) the default export is blacklisted." : `The export "${importName}" is blacklisted.`; } - public static FAILURE_STRING_REGEX = "This import is blacklisted by "; - public isEnabled(): boolean { return super.isEnabled() && this.ruleArguments.length > 0; } @@ -105,7 +105,7 @@ export class Rule extends Lint.Rules.AbstractRule { } } -type Options = Array>; +type Options = Array; function walk(ctx: Lint.WalkContext) { interface BannedImports {