diff --git a/projects/igniteui-angular/migrations/common/UpdateChanges.spec.ts b/projects/igniteui-angular/migrations/common/UpdateChanges.spec.ts index 891847cd11f..3f1a28849f3 100644 --- a/projects/igniteui-angular/migrations/common/UpdateChanges.spec.ts +++ b/projects/igniteui-angular/migrations/common/UpdateChanges.spec.ts @@ -645,6 +645,84 @@ $var3: igx-comp-theme( done(); }); + it('should keep the brackets balanced for nested theme functions', done => { + const themeChangesJson: ThemeChanges = { + changes: [ + { + name: '$remove-me', remove: true, + owner: 'igx-theme-func', + type: ThemeType.Property + }, + { + name: '$replace-me', replaceWith: '$replaced', + owner: 'igx-theme-func', + type: ThemeType.Property + } + ] + }; + const jsonPath = path.join(__dirname, 'changes', 'theme-changes.json'); + spyOn(fs, 'existsSync').and.callFake((filePath: fs.PathLike) => filePath === jsonPath); + spyOn(fs, 'readFileSync').and.callFake(() => JSON.stringify(themeChangesJson)); + + appTree.create('styles.scss', +`@include igx-mixin(igx-theme-func($remove-me: 6px)); +@include igx-mixin(igx-theme-func($prop1: red, $remove-me: 6px)); +@include igx-mixin(igx-theme-func($remove-me: 6px, $prop1: red)); +@include igx-mixin(igx-theme-func($replace-me: 6px)); +$var: igx-theme-func($content: "not a ( bracket", $remove-me: 6px); +$var2: igx-theme-func($image: url(https://example.com/a.png), $remove-me: 6px); +$var3: igx-theme-func( + $remove-me: 6px, // not a ) bracket + $prop1: red +);`); + + const update = new UnitUpdateChanges(__dirname, appTree); + update.applyChanges(); + + expect(appTree.readContent('styles.scss')).toEqual( +`@include igx-mixin(igx-theme-func()); +@include igx-mixin(igx-theme-func($prop1: red)); +@include igx-mixin(igx-theme-func( $prop1: red)); +@include igx-mixin(igx-theme-func($replaced: 6px)); +$var: igx-theme-func($content: "not a ( bracket"); +$var2: igx-theme-func($image: url(https://example.com/a.png)); +$var3: igx-theme-func( // not a ) bracket + $prop1: red +);`); + done(); + }); + + it('should not treat a theme function mentioned in a comment or a string as a call', done => { + const themeChangesJson: ThemeChanges = { + changes: [ + { + name: '$remove-me', remove: true, + owner: 'igx-theme-func', + type: ThemeType.Property + } + ] + }; + const jsonPath = path.join(__dirname, 'changes', 'theme-changes.json'); + spyOn(fs, 'existsSync').and.callFake((filePath: fs.PathLike) => filePath === jsonPath); + spyOn(fs, 'readFileSync').and.callFake(() => JSON.stringify(themeChangesJson)); + + appTree.create('styles.scss', +`// igx-theme-func($remove-me: 1px) is gone, use the grid's own borders +/* igx-theme-func($remove-me: 2px */ +$doc: "igx-theme-func($remove-me: 3px"; +$var: igx-theme-func($remove-me: 4px, $prop1: red);`); + + const update = new UnitUpdateChanges(__dirname, appTree); + update.applyChanges(); + + expect(appTree.readContent('styles.scss')).toEqual( +`// igx-theme-func($remove-me: 1px) is gone, use the grid's own borders +/* igx-theme-func($remove-me: 2px */ +$doc: "igx-theme-func($remove-me: 3px"; +$var: igx-theme-func( $prop1: red);`); + done(); + }); + it('should replace imports', done => { const importsJson: ImportsChanges = { changes: [ diff --git a/projects/igniteui-angular/migrations/common/UpdateChanges.ts b/projects/igniteui-angular/migrations/common/UpdateChanges.ts index 9479ce34b1e..9e89789f5fa 100644 --- a/projects/igniteui-angular/migrations/common/UpdateChanges.ts +++ b/projects/igniteui-angular/migrations/common/UpdateChanges.ts @@ -397,44 +397,44 @@ export class UpdateChanges { if (change.type !== ThemeType.Property) { continue; } - if (fileContent.indexOf(change.owner) !== -1) { - /** owner-func:( * ); */ - const searchPattern = String.raw`${change.owner}\([\s\S]+?\);`; - const matches = fileContent.match(new RegExp(searchPattern, 'g')); - if (!matches) { + if (fileContent.indexOf(change.owner) === -1) { + continue; + } + /** owner-func:( * ) */ + const calls = this.findFunctionCalls(fileContent, change.owner); + // rewrite back to front so the collected indices stay valid + for (const call of calls.reverse()) { + const rawBody = fileContent.substring(call.bodyStart, call.bodyEnd); + if (rawBody.indexOf(change.name) === -1) { continue; } - for (const match of matches) { - if (match.indexOf(change.name) !== -1) { - const name = change.name.replace('$', '\\$'); - const replaceWith = change.replaceWith?.replace('$', '\\$'); - const reg = new RegExp(String.raw`^\s*${name}:`); - const existing = new RegExp(String.raw`${replaceWith}:`); - const opening = `${change.owner}(`; - const closing = /\s*\);$/.exec(match).pop(); - const body = match.substr(opening.length, match.length - opening.length - closing.length); - - let params = this.splitFunctionProps(body); - params = params.reduce((arr, param) => { - if (reg.test(param)) { - const duplicate = !!replaceWith && arr.some(p => existing.test(p)); - - if (!change.remove && !duplicate) { - arr.push(param.replace(change.name, change.replaceWith)); - } - } else { - arr.push(param); - } - return arr; - }, []); - - fileContent = fileContent.replace( - match, - opening + params.join(',') + closing - ); - overwrite = true; + const name = escapeRegExp(change.name); + const replaceWith = change.replaceWith ? escapeRegExp(change.replaceWith) : undefined; + const reg = new RegExp(String.raw`^\s*${name}:`); + const existing = new RegExp(String.raw`${replaceWith}:`); + // keep whatever sits in front of the closing bracket so the formatting is preserved + const trailing = /\s*$/.exec(rawBody).pop(); + const body = rawBody.substring(0, rawBody.length - trailing.length); + + let params = this.splitFunctionProps(body); + params = params.reduce((arr, param) => { + if (reg.test(param)) { + const duplicate = !!replaceWith && arr.some(p => existing.test(p)); + + if (!change.remove && !duplicate) { + arr.push(param.replace(change.name, change.replaceWith)); + } + } else { + arr.push(param); } - } + return arr; + }, []); + + fileContent = fileContent.substring(0, call.bodyStart) + + params.join(',') + + trailing + + fileContent.substring(call.bodyEnd); + overwrite = true; } } if (overwrite) { @@ -442,6 +442,120 @@ export class UpdateChanges { } } + /** + * Returns the argument list boundaries of every top-level `owner(...)` call in the content. + * Strings and comments are scanned over, so an `owner(` that is only mentioned in one is not + * taken for a call. The brackets are tracked too, so a call nested in another one - + * `@include scrollbar(scrollbar-theme($sb-size: 6px))` - reports its own closing bracket + * rather than the one of the call surrounding it. + */ + private findFunctionCalls(content: string, owner: string): { bodyStart: number; bodyEnd: number }[] { + const calls: { bodyStart: number; bodyEnd: number }[] = []; + const opening = `${owner}(`; + + for (let i = 0; i < content.length; i++) { + const nonCodeEnd = this.skipNonCode(content, i); + if (nonCodeEnd !== -1) { + i = nonCodeEnd; + continue; + } + if (!content.startsWith(opening, i)) { + continue; + } + + const bodyStart = i + opening.length; + const bodyEnd = this.findClosingBracket(content, bodyStart); + if (bodyEnd === -1) { + // unbalanced content, nothing safe left to rewrite + break; + } + + calls.push({ bodyStart, bodyEnd }); + // a same-owner call nested in this one is already covered by it + i = bodyEnd; + } + + return calls; + } + + /** + * Returns the index of the bracket closing the one `start` is inside of, or -1 when the + * content is unbalanced. Brackets in strings and comments are ignored. + */ + private findClosingBracket(content: string, start: number): number { + let level = 0; + + for (let i = start; i < content.length; i++) { + const nonCodeEnd = this.skipNonCode(content, i); + if (nonCodeEnd !== -1) { + i = nonCodeEnd; + continue; + } + + const char = content[i]; + if (char === '(') { + level++; + } else if (char === ')') { + if (!level) { + return i; + } + level--; + } + } + + return -1; + } + + /** + * When a string or a comment starts at `index`, returns the index of its last character so + * that a scan can carry on past it. Returns -1 when `index` is on code. + */ + private skipNonCode(content: string, index: number): number { + const char = content[index]; + const next = content[index + 1]; + + if (char === '\'' || char === '"') { + return this.skipString(content, index); + } + if (char === '/' && next === '*') { + const end = content.indexOf('*/', index + 2); + + return end === -1 ? content.length : end + 1; + } + if (char === '/' && next === '/' && this.isLineCommentStart(content, index)) { + const end = content.indexOf('\n', index + 2); + + return end === -1 ? content.length : end; + } + + return -1; + } + + /** + * Tells apart a `//` line comment from the `//` of a protocol - `url(https://...)` - + * by looking at what precedes it. + */ + private isLineCommentStart(content: string, index: number): boolean { + const previous = content[index - 1]; + + return previous === undefined || /[\s,(;{]/.test(previous); + } + + /** Returns the index of the quote closing the string opened at `start`. */ + private skipString(content: string, start: number): number { + const quote = content[start]; + + for (let i = start + 1; i < content.length; i++) { + if (content[i] === '\\') { + i++; + } else if (content[i] === quote) { + return i; + } + } + + return content.length; + } + protected isNamedArgument(fileContent: string, i: number, occurrences: number[], change: ThemeChange) { const openingBrackets = []; const closingBrackets = []; @@ -879,6 +993,10 @@ export class UpdateChanges { for (let i = 0; i < body.length; i++) { const char = body[i]; + if (char === '\'' || char === '"') { + i = this.skipString(body, i); + continue; + } switch (char) { case '(': level++; break; case ')': level--; break; diff --git a/projects/igniteui-angular/migrations/update-22_2_0/index.spec.ts b/projects/igniteui-angular/migrations/update-22_2_0/index.spec.ts index aa5b7bdaf7e..93b6d56c6c8 100644 --- a/projects/igniteui-angular/migrations/update-22_2_0/index.spec.ts +++ b/projects/igniteui-angular/migrations/update-22_2_0/index.spec.ts @@ -50,6 +50,49 @@ describe(`Update to ${version}`, () => { expect(tree.readContent('/testSrc/appPrefix/component/test.component.scss')).toEqual(content); }); + it('should keep the brackets balanced when the theme is nested in another call', async () => { + appTree.create( + `/testSrc/appPrefix/component/test.component.scss`, + `.selection-area { + @include scrollbar(scrollbar-theme($sb-size: 6px)); +} + +igx-grid { + @include scrollbar(scrollbar-theme($sb-thumb-bg-color: blue, $sb-size: 16px)); +}` + ); + + const tree = await schematicRunner.runSchematic(migrationName, { shouldInvokeLS: false }, appTree); + + expect(tree.readContent('/testSrc/appPrefix/component/test.component.scss')).toEqual( + `.selection-area { + @include scrollbar(scrollbar-theme()); +} + +igx-grid { + @include scrollbar(scrollbar-theme($sb-thumb-bg-color: blue)); +}` + ); + }); + + it('should migrate a theme call that is not terminated by a semicolon', async () => { + appTree.create( + `/testSrc/appPrefix/component/test.component.scss`, + `$my-scrollbar: scrollbar-theme( + $sb-size: 16px, + $sb-thumb-bg-color: blue +)` + ); + + const tree = await schematicRunner.runSchematic(migrationName, { shouldInvokeLS: false }, appTree); + + expect(tree.readContent('/testSrc/appPrefix/component/test.component.scss')).toEqual( + `$my-scrollbar: scrollbar-theme( + $sb-thumb-bg-color: blue +)` + ); + }); + it('should not touch same-named properties on other themes', async () => { const content = `$my-grid: grid-theme( $sb-size: 16px