Skip to content

Commit

Permalink
Merge pull request #2696 from ember-template-lint/reproduce-gts-fix-i…
Browse files Browse the repository at this point in the history
…ssue

Co-authored-by: Gabriel Csapo <[email protected]>
Co-authored-by: Robert Jackson <[email protected]>
  • Loading branch information
rwjblue and gabrielcsapo authored Nov 17, 2022
2 parents 233e9de + f5abdf0 commit a691006
Show file tree
Hide file tree
Showing 9 changed files with 334 additions and 92 deletions.
13 changes: 9 additions & 4 deletions lib/extract-templates.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { parseTemplates } from 'ember-template-imports/lib/parse-templates.js';
import * as util from 'ember-template-imports/src/util.js';

export const SUPPORTED_EXTENSIONS = ['.js', '.ts', '.gjs', '.gts'];
const LOCATION_START = Object.freeze({ line: 0, column: 0 });
const LOCATION_START = Object.freeze({ line: 0, column: 0, start: 0, end: 0 });
/**
* Processes results and corrects for template location offsets.
*
Expand Down Expand Up @@ -66,7 +66,7 @@ export function extractTemplates(moduleSource, relativePath) {
let templateStart = start.index + start[0].length;

return makeTemplateInfo(
coordinatesOf(moduleSource, templateStart, end.index),
coordinatesOf(moduleSource, templateStart, start.index, end.index),
templateInfo.contents,
templateInfo,
true
Expand All @@ -86,13 +86,16 @@ export function extractTemplates(moduleSource, relativePath) {
*
* @returns {TemplateInfo}
*/
function makeTemplateInfo({ line, column }, template, templateInfo, isEmbedded) {
function makeTemplateInfo({ line, column, start, end }, template, templateInfo, isEmbedded) {
return {
line,
start,
end,
column,
template,
isEmbedded,
isStrictMode: !isEmbedded || isStrictMode(templateInfo),
templateMatch: templateInfo,
};
}

Expand All @@ -113,11 +116,13 @@ export function isSupportedScriptFileExtension(filePath = '') {
return SUPPORTED_EXTENSIONS.some((ext) => filePath.endsWith(ext));
}

export function coordinatesOf(text, offset) {
export function coordinatesOf(text, offset, start, end) {
const contentBeforeTemplateStart = text.slice(0, offset).split('\n');
return {
line: contentBeforeTemplateStart.length,
column: contentBeforeTemplateStart[contentBeforeTemplateStart.length - 1].length,
start,
end,
};
}

Expand Down
11 changes: 8 additions & 3 deletions lib/helpers/rule-test-harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const ALLOWED_TEST_CASE_PROPERTIES_BAD = new Set([
'results',
'template',
'verifyResults',
'skipDisabledTests',
]);

const ALLOWED_TEST_CASE_PROPERTIES_ERROR = new Set([
Expand Down Expand Up @@ -407,6 +408,8 @@ export default function generateRuleTests(...args) {
for (const item of bad) {
let template = item.template;

let shouldRunDisabledTests =
'skipDisabledTests' in item ? !item.skipDisabledTests : !skipDisabledTests;
let shouldFocus = item.focus === true && typeof focusMethod === 'function';
let testOrOnly = shouldFocus ? focusMethod : testMethod;
let testName = item.name ? item.name : template;
Expand Down Expand Up @@ -469,7 +472,7 @@ export default function generateRuleTests(...args) {
}
});

if (!skipDisabledTests) {
if (shouldRunDisabledTests) {
testOrOnly(`${testName}: passes when rule is disabled`, async function () {
checkLinterReuse();

Expand Down Expand Up @@ -521,7 +524,8 @@ export default function generateRuleTests(...args) {
testOrOnly(`\`${item.template}\` -> ${item.fixedTemplate}\``, async function () {
checkLinterReuse();

let options = await prepare(item.template, item.config);
let options = await prepare(item, item.config);

let result = await linter.verifyAndFix(options);

assert.deepStrictEqual(result.output, item.fixedTemplate);
Expand All @@ -530,7 +534,8 @@ export default function generateRuleTests(...args) {
testOrOnly(`${item.fixedTemplate}\` is idempotent`, async function () {
checkLinterReuse();

let options = await prepare(item.fixedTemplate, item.config);
let options = await prepare(item, item.config);
options.source = item.fixedTemplate;
let result = await linter.verifyAndFix(options);

assert.deepStrictEqual(result.output, item.fixedTemplate);
Expand Down
60 changes: 49 additions & 11 deletions lib/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,28 +219,66 @@ export default class Linter {
* @returns {Promise<string>} output - The fixed source.
*/
async _applyFixes(options, results) {
let currentSource = options.source;
let fixableIssues = results.filter((r) => r.isFixable);

// nothing to do, bail out
if (fixableIssues.length === 0) {
return options.source;
return currentSource;
}

let currentSource = options.source;
let fileConfig = this._moduleStatusCache.getConfigForFile(options);

let ruleNames = new Set(fixableIssues.map((r) => r.rule));

for (let ruleName of ruleNames) {
let rule = this._buildRule(ruleName, {
shouldFix: true,
filePath: options.filePath,
rawSource: currentSource,
fileConfig,
});
let templateInfos = extractTemplates(currentSource, options.filePath);

for (let templateInfo of templateInfos) {
let { templateMatch } = templateInfo;

let visitor = await rule.getVisitor();
let { code } = transform(currentSource, () => visitor);
currentSource = code;
let rule = this._buildRule(ruleName, {
shouldFix: true,
filePath: options.filePath,
rawSource: templateInfo.template,
fileConfig,
});

let visitor = await rule.getVisitor();
let { code } = transform(templateInfo.template, () => visitor);

if (code !== templateInfo.template) {
let prefix;
if (!templateInfo.templateMatch) {
// this is a `.hbs` file, no prefix needed
prefix = '';
} else if (templateInfo.templateMatch.type === 'template-tag') {
// handing `<template></template>`
let { start } = templateMatch;

prefix = currentSource.slice(start.index, start.index + start[0].length);
} else {
// handling things like hbs`Hello!`
prefix = `${templateMatch.tagName}\``;
}

// if there is no prefix we don't need to slice anything, just use the code output by transform
currentSource = prefix
? currentSource.slice(0, templateInfo.start) +
prefix +
code +
currentSource.slice(templateInfo.end)
: code;

return await this._applyFixes(
{
...options,
source: currentSource,
},
results
);
}
}
}

return currentSource;
Expand Down
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
"chalk": "^4.1.2",
"ci-info": "^3.4.0",
"date-fns": "^2.29.2",
"ember-template-imports": "^3.2.0",
"ember-template-imports": "^3.4.0",
"ember-template-recast": "^6.1.3",
"find-up": "^6.3.0",
"fuse.js": "^6.5.3",
Expand Down
22 changes: 22 additions & 0 deletions test/acceptance/cli-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1249,5 +1249,27 @@ app/dist/application.hbs

expect(fileContents).toEqual('<button type="button">Klikk</button>');
});

it('should support writing a `.gts` file to fs', async function () {
let config = { rules: { 'require-button-type': true } };
await project.setConfig(config);
await project.write({
'require-button-type.gts': `/** Some docs here: Use this <button> like blah blah */ <template><button>Klikk</button></template>`,
});

let result = await runBin('./require-button-type.gts', '--fix');

expect(result.stdout).toBeFalsy();
expect(result.stderr).toBeFalsy();
expect(result.exitCode).toEqual(0);

let fileContents = fs.readFileSync(project.path('require-button-type.gts'), {
encoding: 'utf8',
});

expect(fileContents).toEqual(
`/** Some docs here: Use this <button> like blah blah */ <template><button type="button">Klikk</button></template>`
);
});
});
});
2 changes: 1 addition & 1 deletion test/acceptance/rule-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ describe('rule public api', function () {
bad: [{ foo: true }],
})
).toThrowErrorMatchingInlineSnapshot(
`"Unexpected property passed to bad test case: foo. Expected one of: config, fixedTemplate, meta, name, result, results, template, verifyResults."`
`"Unexpected property passed to bad test case: foo. Expected one of: config, fixedTemplate, meta, name, result, results, template, verifyResults, skipDisabledTests."`
);
});

Expand Down
Loading

0 comments on commit a691006

Please sign in to comment.