From 0ae4187566bb6e0121b0026cb897f00a328d2aa8 Mon Sep 17 00:00:00 2001 From: dario-piotrowicz Date: Wed, 13 Oct 2021 22:13:44 +0100 Subject: [PATCH] refactor(docs-infra): remove eslint directive comments from examples (#43831) Linting has recently been removed from the examples provided in angular.io (see PRs #43592 and #43746) such removal effects the downloadable and stackblitz examples but linting is still generally used in the examples in the aio project itself (they are being migrated from tslint to eslint in PR #43218) thus eslint directive comments are still necessary in the code itself. So the comments need to be present but need not to be exposed to the users (not in the zips, stackblitzes nor docs themselves), these changes are removing such comments during the examples' parsing phase (effectively removing them from all three sources mentioned above). Original discussion: https://github.com/angular/angular/pull/43218#discussion_r697305494 resolves #43788 PR Close #43831 --- .../services/region-parser.js | 3 +- .../services/removeEslintComments.js | 32 ++ .../services/removeEslintComments.spec.js | 322 ++++++++++++++++++ 3 files changed, 356 insertions(+), 1 deletion(-) create mode 100644 aio/tools/transforms/examples-package/services/removeEslintComments.js create mode 100644 aio/tools/transforms/examples-package/services/removeEslintComments.spec.js diff --git a/aio/tools/transforms/examples-package/services/region-parser.js b/aio/tools/transforms/examples-package/services/region-parser.js index 465218324f6..cdbd2c2f46a 100644 --- a/aio/tools/transforms/examples-package/services/region-parser.js +++ b/aio/tools/transforms/examples-package/services/region-parser.js @@ -5,6 +5,7 @@ const inlineCOnly = require('./region-matchers/inline-c-only'); const inlineHash = require('./region-matchers/inline-hash'); const DEFAULT_PLASTER = '. . .'; const {mapObject} = require('../../helpers/utils'); +const removeEslintComments = require('./removeEslintComments'); module.exports = function regionParser() { regionParserImpl.regionMatchers = { @@ -40,7 +41,7 @@ module.exports = function regionParser() { if (regionMatcher) { let plaster = regionMatcher.createPlasterComment(DEFAULT_PLASTER); - const lines = contents.split(/\r?\n/).filter((line, index) => { + const lines = removeEslintComments(contents, fileType).split(/\r?\n/).filter((line, index) => { const startRegion = line.match(regionMatcher.regionStartMatcher); const endRegion = line.match(regionMatcher.regionEndMatcher); const updatePlaster = line.match(regionMatcher.plasterMatcher); diff --git a/aio/tools/transforms/examples-package/services/removeEslintComments.js b/aio/tools/transforms/examples-package/services/removeEslintComments.js new file mode 100644 index 00000000000..08b1efee0df --- /dev/null +++ b/aio/tools/transforms/examples-package/services/removeEslintComments.js @@ -0,0 +1,32 @@ +module.exports = function removeEslintComments(input, fileType) { + const regexForFileType = regexesForFileTypes[fileType]; + + if (!input || !regexForFileType) { + return input; + } + + return input.replace(regexForFileType, ''); +}; + +const jsRegexes = [ + /\/\/ *eslint-disable(?:-next-line)?(?: .*)?(?:\n *|$)/, + /\n? *\/\/ *eslint-(?:disable-line|enable)(?: .*)?(?=\n|$)/, + /\/\*\s*eslint-disable(?:-next-line)?(?: [\s\S]*?)?\*\/ *(?:\n *)?/, + /\n? *\/\*\s*eslint-(?:disable-line|enable)(?: [\s\S]*?)?\*\//, +]; + +const htmlRegexes = [ + / *(?:\n *)?/, + /\n? */, +]; + +const joinRegexes = regexes => new RegExp(regexes.map(regex => `(?:${regex.source})`).join('|'), 'g'); +const htmlRegex = joinRegexes(htmlRegexes); +// Note: the js regex needs to also include the html ones to account for inline templates in @Components +const jsRegex = joinRegexes([...jsRegexes, ...htmlRegexes]); + +const regexesForFileTypes = { + js: jsRegex, + ts: jsRegex, + html: htmlRegex, +}; diff --git a/aio/tools/transforms/examples-package/services/removeEslintComments.spec.js b/aio/tools/transforms/examples-package/services/removeEslintComments.spec.js new file mode 100644 index 00000000000..bc2cc899e51 --- /dev/null +++ b/aio/tools/transforms/examples-package/services/removeEslintComments.spec.js @@ -0,0 +1,322 @@ +/* eslint-disable jasmine/no-spec-dupes */ +const removeEslintComments = require('./removeEslintComments'); + +describe('removeEslintComments', () => { + + it('should return the given input if that is null or undefined', () => { + expect(removeEslintComments(null, 'ts')).toEqual(null); + expect(removeEslintComments(undefined, 'html')).toEqual(undefined); + }); + + it('should return the given input if the provided fileType is neither js, ts nor html', () => { + const testText = ` + Simple text containing comments js comments like: + /* eslint-disable */ + and html ones like: + + `; + expect(removeEslintComments(testText)).toEqual(testText); + expect(removeEslintComments(testText, 'css')).toEqual(testText); + expect(removeEslintComments(testText, 'json')).toEqual(testText); + }); + + describe('js and ts', () => { + const rmv = source => removeEslintComments(source, 'ts'); + + it('should remove correctly eslint-disable comments', () => { + let source = `/* eslint-disable @angular-eslint/no-input-rename, + @angular-eslint/no-inputs-metadata-property, + @angular-eslint/no-output-rename, + @angular-eslint/no-outputs-metadata-property */ + import { Component, EventEmitter, Input, Output } from '@angular/core'; + + @Component({ + `; + expect(rmv(source)).toMatch( + createRegexForMatching( + 'import { Component, EventEmitter, Input, Output } from \'@angular/core\';\n\n {6}@Component({\n {6}' + ) + ); + source = `/* eslint-disable foo */ + var foo = 'foo';`; + expect(rmv(source)).toEqual('var foo = \'foo\';'); + source = `/* eslint-disable foo, bar, baz */ + var fooBarBaz = 'foo, bar and baz';`; + expect(rmv(source)).toEqual('var fooBarBaz = \'foo, bar and baz\';'); + }); + + it('should remove correctly eslint-enable and eslint-disable comments', () => { + const source = ` + /* eslint-disable */ + import { test } from './test'; + if ( test != null ) { + /* eslint-enable */ + `; + expect(rmv(source)).toMatch( + createRegexForMatching( + '\n {8}import { test } from \'./test\';\n {8}if ( test != null ) {\n {6}' + ) + ); + }); + + it('should remove correctly eslint-disable-line comments', () => { + let source = 'var i = 1; // eslint-disable-line no-var'; + expect(rmv(source)).toEqual('var i = 1;'); + source = 'const foo = "foo";// eslint-disable-line'; + expect(rmv(source)).toEqual('const foo = "foo";'); + source = 'const bar = 123; /* eslint-disable-line no-unused-vars */'; + expect(rmv(source)).toEqual('const bar = 123;'); + source = 'var baz = false; /* eslint-disable-line no-unused-vars, no-var */'; + expect(rmv(source)).toEqual('var baz = false;'); + }); + + it('should remove correctly eslint-disable-next-line comments', () => { + let source = `// eslint-disable-next-line @typescript-eslint/quotes + const string = "string test";`; + expect(rmv(source)).toEqual('const string = "string test";'); + source = '// eslint-disable-next-line foo'; + expect(rmv(source)).toEqual(''); + source = `// eslint-disable-next-line no-console, quotes + console.log("log test");`; + expect(rmv(source)).toEqual('console.log("log test");'); + }); + + it('should remove correctly a mix of different types of eslint comments', () => { + let source = ` + /* eslint-disable */ + var mixed1 = 'test'; + /* eslint-enable */ + mixed1 = "test1"; // eslint-disable-line semi, quotes + + // eslint-disable-next-line eqeqeq + return mixed1 == 'test'; + `; + expect(rmv(source)).toMatch( + createRegexForMatching( + '\n {6}var mixed1 = \'test\';\n {6}mixed1 = "test1";\n\n {6}return mixed1 == \'test\';\n {6}' + ) + ); + source = ` + /* eslint-enable *//* eslint-disable */ + var mixed2 = "test"; // eslint-disable-line + /* eslint-disable */ + // eslint-disable-next-line + console.log(mixed2); + /* eslint-enable */ + `; + expect(rmv(source)).toMatch( + createRegexForMatching('var mixed2 = "test";\n {6}console.log(mixed2);\n {6}') + ); + }); + + it('should handle any number of spaces around the eslint directive', () => { + let source = `// eslint-disable-next-line no-vars + var v1 = 123; + `; + expect(rmv(source)).toMatch(createRegexForMatching('var v1 = 123;\n {6}')); + source = 'var v2 = 234;//eslint-disable-line '; + expect(rmv(source)).toEqual('var v2 = 234;'); + source = 'var v3 = 345;// eslint-disable-line no-vars'; + expect(rmv(source)).toEqual('var v3 = 345;'); + source = ` /* eslint-disable no-vars */ + var v4 = 456; + /*eslint-enable no-vars*/ + `; + expect(rmv(source)).toMatch(createRegexForMatching(' {5}var v4 = 456;\n {8}')); + source = `/*eslint-disable foo*/ + var v5 = 567;`; + expect(rmv(source)).toEqual('var v5 = 567;'); + }); + + it('should ignore generic comments containing eslint terms', () => { + let source = ` + /* this is not an eslint-disable comment */ + // and this is not an eslint-disable-line comment + // this isn't an eslint comment at all + `; + expect(rmv(source)).toEqual(source); + }); + + it('should ignore incorrect eslint comments', () => { + const source = ` + /* eslint-disable-nonsense */ + // eslint-disable-next line + // eslint-disable-next-line-nonsense + // eslint-disable-line-nonsense + // eslint-disable--next-line + /* eslint disable */ + // eslint-enable-line + // eslint-enable-next-line + `; + expect(rmv(source)).toEqual(source); + }); + + it('should remove html eslint comments', () => { + const source = ` + import { Component, OnInit, Input } from '@angular/core'; + + @Component({ + selector: 'app-test', + template: \` + +

+ run eslint test +

+ \`, + styles: [] + }) + export class TestComponent implements OnInit { + @Input() eslintTest: () => {}; + `; + expect(rmv(source)).toMatch( + createRegexForMatching( + '\n {8}import { Component, OnInit, Input } from \'@angular/core\';\n\n {8}@Component({\n' + + ' {10}selector: \'app-test\',\n {10}template: `\n' + + ' {12}

\n' + + ' {14}run eslint test\n {12}

\n {10}`,\n {10}styles: []\n {8}})\n' + + ' {8}export class TestComponent implements OnInit {\n' + + ' {10}@Input() eslintTest: () => {};\n {6}' + ) + ); + }); + + it('should remove correctly eslint directive comments containing descriptions', () => { + let source = `// eslint-disable-next-line a-rule, another-rule -- those are buggy!! + const aRule = "another-rule";`; + expect(rmv(source)).toEqual('const aRule = "another-rule";'); + source = `/* eslint-disable max-len, no-var -- + Disabling the rule is necessary here because we have + variables declared with var which contain very long strings + */ + + export class Utils {`; + expect(rmv(source)).toMatch(createRegexForMatching('\n {6}export class Utils {')); + }); + + }); + + describe('html', () => { + const rmv = source => removeEslintComments(source, 'html'); + + it('should remove correctly eslint-disable comments', () => { + const source = ` + `; + expect(rmv(source)).toEqual(''); + }); + + it('should remove correctly eslint-enable and eslint-disable comments', () => { + const source = ` + + `; + expect(rmv(source)).toEqual(''); + }); + + it('should remove correctly eslint-disable-line comments', () => { + let source = 'eight '; + expect(rmv(source)).toEqual('eight'); + source = 'nine'; + expect(rmv(source)).toEqual('nine'); + source = `
+ +
`; + expect(rmv(source)).toMatch(createRegexForMatching('
\n {8}\n {6}
')); + }); + + it('should remove correctly eslint-disable-next-line comments', () => { + const source = ` + disable-next-line link`; + expect(rmv(source)).toEqual('disable-next-line link'); + }); + + it('should remove correctly a mix of different types of eslint comments', () => { + let source = ` + + + mixed1 + + `; + expect(rmv(source)).toMatch(createRegexForMatching('\n {6} mixed1 \n {6}')); + source = ` + + mixed2 + `; + expect(rmv(source)).toMatch(createRegexForMatching('\n {6} mixed2 \n {6}')); + }); + + it('should handle any number of spaces around the eslint directive', () => { + let source = ''; + expect(rmv(source)).toEqual(''); + source = ` +
click me
`; + expect(rmv(source)).toEqual('
click me
'); + source = ` + `; + expect(rmv(source)).toEqual(''); + }); + + it('should ignore generic comments containing eslint terms', () => { + let source = '

{{ text }}

'; + expect(rmv(source)).toEqual(source); + source = ` + + + `; + expect(rmv(source)).toMatch( + createRegexForMatching( + '\n {8}' + ) + ); + source = ` +

Foo, Bar and Baz

+ `; + expect(rmv(source)).toEqual('

Foo, Bar and Baz

'); + }); + + it('should ignore incorrect eslint comments', () => { + const source = ` + + + + + + + + + `; + expect(rmv(source)).toEqual(source); + }); + + it('should remove correctly eslint directive comments containing descriptions', () => { + let source = ` + violating foo`; + expect(rmv(source)).toEqual('violating foo'); + source = ` + + +
`; + expect(rmv(source)).toMatch(createRegexForMatching('\n {6}\n {6}
')); + }); + + }); + +}); + +/** + * Creates a regex based on a provided string, it performs a partial escape + * of its special characters excluding curly braces surrounding a number so that it + * converts a string such as "if(test) {3}{ }" to the regex /if\(test\) {3}\{ \}/, + * it also surrounds the regex with ^ and $ (for stricter checkings) + * + * @param str input string + * @returns output regex + */ +function createRegexForMatching(str) { + const partiallyEscapedString = str.replace(/(?:[.?*+\\|^$()[\]])|{(?!\d+})|(?