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

Add autofixer to no-trailing-spaces rule #2894

Merged
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ Each rule has emojis denoting:
| [no-scope-outside-table-headings](./docs/rule/no-scope-outside-table-headings.md) | ✅ | | ⌨️ | |
| [no-shadowed-elements](./docs/rule/no-shadowed-elements.md) | ✅ | | | |
| [no-this-in-template-only-components](./docs/rule/no-this-in-template-only-components.md) | | | | 🔧 |
| [no-trailing-spaces](./docs/rule/no-trailing-spaces.md) | | 💅 | | |
| [no-trailing-spaces](./docs/rule/no-trailing-spaces.md) | | 💅 | | 🔧 |
| [no-triple-curlies](./docs/rule/no-triple-curlies.md) | ✅ | | | |
| [no-unbalanced-curlies](./docs/rule/no-unbalanced-curlies.md) | ✅ | | | |
| [no-unbound](./docs/rule/no-unbound.md) | ✅ | | | |
Expand Down
2 changes: 2 additions & 0 deletions docs/rule/no-trailing-spaces.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

💅 The `extends: 'stylistic'` property in a configuration file enables this rule.

🔧 The `--fix` option on the command line can automatically fix some of the problems reported by this rule.

Disallow trailing whitespace at the end of lines.

## Examples
Expand Down
64 changes: 46 additions & 18 deletions lib/rules/no-trailing-spaces.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,59 @@
import Rule from './_base.js';
import { builders as b } from 'ember-template-recast';
import replaceNode from '../helpers/replace-node.js';

export default class NoTrailingSpaces extends Rule {
visitor() {
return {
Template: {
// implementation goes here in exit(): in the exit handler, the rule will not
// be called if it has been disabled by any inline comments within the file.
TextNode(node, { parentNode, parentKey }) {
const sourceLines = this.source.join('').split('\n');
const charsSplit = node.chars.split('\n');
const newChars = [...charsSplit];

exit(node) {
let source = this.sourceForNode(node);
let lines = source.split('\n');
for (const [idx, element] of charsSplit.entries()) {
const match = /\s+$/.exec(element);
if (match) {
const lineNum = node.loc.start.line + idx;
const sourceLine = sourceLines[lineNum - 1];
const column = match.index;

for (const [i, line] of lines.entries()) {
let column = line.length - 1;
let isLastLine = i === lines.length - 1;
/**
* We want to make sure this chunk of whitespace is at the end of the line
*/
const endColumn = column + match[0].length;
const isEndOfLine = endColumn === sourceLine.length;

if (line[column] === ' ' && (!isLastLine || column > this.columnOffset)) {
this.log({
message: 'line cannot end with space',
node,
line: i + 1,
column,
source: line,
});
/**
* We also want to make sure that if this is the last line, it's not just the column offset whitespace
* This will prevent things like the last line of an embedded template from being flagged:
* await render(hbs`
* <div></div>
* `); // this line won't get flagged
*/
const isLastLine = lineNum === sourceLines.length;
const isNotLastLineOffset = !isLastLine || column > this.columnOffset;

if (isEndOfLine && isNotLastLineOffset) {
if (this.mode === 'fix') {
newChars[idx] = element.replace(/\s+$/g, '');
} else {
this.log({
message: 'line cannot end with space',
node,
line: lineNum,
column,
isFixable: true,
source: element,
});
}
}
}
},
}

if (this.mode === 'fix') {
const newNode = b.text(newChars.join('\n'));
replaceNode(node, parentNode, parentKey, newNode);
}
},
};
}
Expand Down
109 changes: 63 additions & 46 deletions test/unit/rules/no-trailing-spaces-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,21 @@ generateRuleTests({

good: [
'test',
' test',
'test\n',
'test\n' + '\n',
// test the re-entering of yielded content
'{{#my-component}}\n' + ' test\n' + '{{/my-component}}',
{
template: [
"import { hbs } from 'ember-cli-htmlbars';",
'',
"test('it renders', async (assert) => {",
' await render(hbs`',
' <div class="parent">',
' <div class="child"></div>',
' </div>',
' `);',
');',
].join('\n'),
template: `import { hbs } from 'ember-cli-htmlbars';

test('it renders', async (assert) => {
await render(hbs\`
<div class="parent">
<div class="child"></div>
</div>
\`);
);`,
meta: {
filePath: 'layout.js',
},
Expand All @@ -32,7 +31,7 @@ generateRuleTests({
bad: [
{
template: 'test ',

fixedTemplate: 'test',
verifyResults(results) {
expect(results).toMatchInlineSnapshot(`
[
Expand All @@ -41,6 +40,7 @@ generateRuleTests({
"endColumn": 5,
"endLine": 1,
"filePath": "layout.hbs",
"isFixable": true,
"line": 1,
"message": "line cannot end with space",
"rule": "no-trailing-spaces",
Expand All @@ -53,7 +53,7 @@ generateRuleTests({
},
{
template: 'test \n',

fixedTemplate: 'test\n',
verifyResults(results) {
expect(results).toMatchInlineSnapshot(`
[
Expand All @@ -62,6 +62,7 @@ generateRuleTests({
"endColumn": 0,
"endLine": 2,
"filePath": "layout.hbs",
"isFixable": true,
"line": 1,
"message": "line cannot end with space",
"rule": "no-trailing-spaces",
Expand All @@ -74,7 +75,7 @@ generateRuleTests({
},
{
template: 'test\n' + ' \n',

fixedTemplate: 'test\n' + '\n',
verifyResults(results) {
expect(results).toMatchInlineSnapshot(`
[
Expand All @@ -83,6 +84,7 @@ generateRuleTests({
"endColumn": 0,
"endLine": 3,
"filePath": "layout.hbs",
"isFixable": true,
"line": 2,
"message": "line cannot end with space",
"rule": "no-trailing-spaces",
Expand All @@ -97,15 +99,16 @@ generateRuleTests({
// only generates one error instead of two
{
template: '{{#my-component}}\n' + ' test \n' + '{{/my-component}}',

fixedTemplate: '{{#my-component}}\n' + ' test\n' + '{{/my-component}}',
verifyResults(results) {
expect(results).toMatchInlineSnapshot(`
[
{
"column": 6,
"endColumn": 17,
"endColumn": 0,
"endLine": 3,
"filePath": "layout.hbs",
"isFixable": true,
"line": 2,
"message": "line cannot end with space",
"rule": "no-trailing-spaces",
Expand All @@ -117,29 +120,36 @@ generateRuleTests({
},
},
{
template: [
"import { hbs } from 'ember-cli-htmlbars';",
'',
"test('it renders', async (assert) => {",
' await render(hbs` ',
' <div class="parent">',
' <div class="child"></div>',
' </div>',
' `);',
');',
].join('\n'),
template: `import { hbs } from 'ember-cli-htmlbars';

test('it renders', async (assert) => {
await render(hbs\`
<div class="parent">
<div class="child"></div>
</div>
\`);
);`,
fixedTemplate: `import { hbs } from 'ember-cli-htmlbars';

test('it renders', async (assert) => {
await render(hbs\`
<div class="parent">
<div class="child"></div>
</div>
\`);
);`,
meta: {
filePath: 'layout.js',
},

verifyResults(results) {
expect(results).toMatchInlineSnapshot(`
[
{
"column": 20,
"endColumn": 21,
"endLine": 8,
"column": 19,
"endColumn": 23,
"endLine": 5,
"filePath": "layout.js",
"isFixable": true,
"line": 4,
"message": "line cannot end with space",
"rule": "no-trailing-spaces",
Expand All @@ -151,29 +161,36 @@ generateRuleTests({
},
},
{
template: [
"import { hbs } from 'ember-cli-htmlbars';",
'',
"test('it renders', async (assert) => {",
' await render(hbs`',
' <div></div>',
' ',
' <div></div>',
' `);',
');',
].join('\n'),
template: `import { hbs } from 'ember-cli-htmlbars';

test('it renders', async (assert) => {
await render(hbs\`
<div></div>

<div></div>
\`);
);`,
fixedTemplate: `import { hbs } from 'ember-cli-htmlbars';

test('it renders', async (assert) => {
await render(hbs\`
<div></div>

<div></div>
\`);
);`,
meta: {
filePath: 'layout.js',
},

verifyResults(results) {
expect(results).toMatchInlineSnapshot(`
[
{
"column": 1,
"endColumn": 2,
"endLine": 8,
"column": 0,
"endColumn": 4,
"endLine": 7,
"filePath": "layout.js",
"isFixable": true,
"line": 6,
"message": "line cannot end with space",
"rule": "no-trailing-spaces",
Expand Down