Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support autofix in gts files #1853

Merged
merged 2 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions lib/preprocessors/glimmer.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ function mapRange(messages, filename) {
return flattened;
}

const originalDoc = new DocumentLines(original);
const transformedDoc = new DocumentLines(transformed);

const lines = transformed.split('\n');
Expand Down Expand Up @@ -193,6 +194,19 @@ function mapRange(messages, filename) {
});

return filtered.map((message) => {
// we need to adjust the fix range provided by eslint to the correct range in the original content
// the lines & columns did not change, except for the template part. but the total offset did, as the template content changed
// therefore we need to remap the range correctly:
// 1. convert range in transformed content to line & column. We know this is the same, but offsets are different
// 2. convert line & column back to offsets in the original content
if (message.fix) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add any high-level comment to provide some context about our support for autofixing and how it works?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

const [start, end] = message.fix.range;
const startPos = transformedDoc.offsetToPosition(start);
const endPos = transformedDoc.offsetToPosition(end);
const fix = message.fix;
fix.range = [originalDoc.positionToOffset(startPos), originalDoc.positionToOffset(endPos)];
}

// 1. handle eslint diagnostics on single lines.
if (message.line === message.endLine) {
const originalLine = originalLines[message.line - 1];
Expand Down Expand Up @@ -295,4 +309,5 @@ function mapRange(messages, filename) {
module.exports = {
preprocess: gjs,
postprocess: mapRange,
supportsAutofix: true,
};
3 changes: 1 addition & 2 deletions lib/utils/document.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
/* istanbul ignore file */

/**
* @typedef {{ line: number; character: number }} Position
*/
Expand Down Expand Up @@ -75,6 +73,7 @@ function computeLineStarts(text) {
return result;
}

/* istanbul ignore next */
/**
* @param {number} ch
* @return {boolean}
Expand Down
35 changes: 33 additions & 2 deletions tests/lib/rules-preprocessor/gjs-gts-processor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ function initESLint(options) {
plugins: ['ember'],
extends: ['plugin:ember/recommended'],
rules: {
quotes: ['error', 'single'],
semi: ['error', 'always'],
'object-curly-spacing': ['error', 'always'],
'lines-between-class-members': 'error',
Expand Down Expand Up @@ -173,7 +174,7 @@ const invalid = [
import Component from '@glimmer/component';

export default class MyComponent extends Component {
foo = "bar";
foo = 'bar';
<template>"hi"</template>
}
`,
Expand All @@ -193,7 +194,7 @@ const invalid = [
import Component from '@glimmer/component';

export default class MyComponent extends Component {
foo = "bar";
foo = 'bar';
<template>"hi"
</template>
}
Expand All @@ -208,6 +209,36 @@ const invalid = [
},
],
},
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a way to add the fixed output to this test? is that worth it?

Copy link
Contributor Author

@patricklx patricklx May 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean the "file content" after auto fix? Not sure if that's worth it. That would just test eslint auto fix capability... Just need to be sure that the fix range is correct.

filename: 'my-component.gjs',
code: `
import Component from "@glimmer/component";
export default class MyComponent extends Component {
foo = 'bar';
<template>"hi"
</template>
}
`,
errors: [
{
message: 'Strings must use singlequote.',
line: 2,
endLine: 2,
endColumn: 49,
column: 29,
fix: {
range: [29, 49],
},
},
{
message: 'Expected blank line between class members.',
line: 5,
endLine: 6,
column: 9,
endColumn: 20,
},
],
},
];

describe('template-vars', () => {
Expand Down