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

feat(html-closing-bracket-new-line): add rule #870

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ These rules relate to style guidelines, and are therefore quite subjective:
|:--------|:------------|:---|
| [svelte/derived-has-same-inputs-outputs](https://sveltejs.github.io/eslint-plugin-svelte/rules/derived-has-same-inputs-outputs/) | derived store should use same variable names between values and callback | |
| [svelte/first-attribute-linebreak](https://sveltejs.github.io/eslint-plugin-svelte/rules/first-attribute-linebreak/) | enforce the location of first attribute | :wrench: |
| [svelte/html-closing-bracket-new-line](https://sveltejs.github.io/eslint-plugin-svelte/rules/html-closing-bracket-new-line/) | Require or disallow a line break before tag's closing brackets | :wrench: |
| [svelte/html-closing-bracket-spacing](https://sveltejs.github.io/eslint-plugin-svelte/rules/html-closing-bracket-spacing/) | require or disallow a space before tag's closing brackets | :wrench: |
| [svelte/html-quotes](https://sveltejs.github.io/eslint-plugin-svelte/rules/html-quotes/) | enforce quotes style of HTML attributes | :wrench: |
| [svelte/html-self-closing](https://sveltejs.github.io/eslint-plugin-svelte/rules/html-self-closing/) | enforce self-closing style | :wrench: |
Expand Down
1 change: 1 addition & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ These rules relate to style guidelines, and are therefore quite subjective:
| :------------------------------------------------------------------------------------------------------- | :--------------------------------------------------------------------------------- | :------- |
| [svelte/derived-has-same-inputs-outputs](./rules/derived-has-same-inputs-outputs.md) | derived store should use same variable names between values and callback | |
| [svelte/first-attribute-linebreak](./rules/first-attribute-linebreak.md) | enforce the location of first attribute | :wrench: |
| [svelte/html-closing-bracket-new-line](./rules/html-closing-bracket-new-line.md) | Require or disallow a line break before tag's closing brackets | :wrench: |
| [svelte/html-closing-bracket-spacing](./rules/html-closing-bracket-spacing.md) | require or disallow a space before tag's closing brackets | :wrench: |
| [svelte/html-quotes](./rules/html-quotes.md) | enforce quotes style of HTML attributes | :wrench: |
| [svelte/html-self-closing](./rules/html-self-closing.md) | enforce self-closing style | :wrench: |
Expand Down
89 changes: 89 additions & 0 deletions docs/rules/html-closing-bracket-new-line.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
---
pageClass: 'rule-details'
sidebarDepth: 0
title: 'svelte/html-closing-bracket-new-line'
description: "Require or disallow a line break before tag's closing brackets"
---

# svelte/html-closing-bracket-new-line

> Require or disallow a line break before tag's closing brackets

- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>
- :wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.

## :book: Rule Details

This rule enforces a line break (or no line break) before tag's closing brackets, which can also be configured to be enforced on self-closing tags.

<ESLintCodeBlock fix>

<!-- prettier-ignore-start -->
<!--eslint-skip-->

```svelte
<script>
/* eslint svelte/brackets-same-line: "error" */
Copy link

Choose a reason for hiding this comment

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

svelte/html-closing-bracket-new-line?

</script>

<!-- ✓ GOOD -->
<div></div>
<div
multiline
>
Children
</div>

<SelfClosing />
<SelfClosing
multiline
/>

<!-- ✗ BAD -->

<div
></div>
<div
multiline>
Children
</div>

<SelfClosing
/>
<SelfClosing
multiline/>
```

<!-- prettier-ignore-end -->

</ESLintCodeBlock>

## :wrench: Options

```jsonc
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ota-meshi, jsonc feels like the right lang to use, does it have any issue with the docs syntax highlighting?

Copy link
Member

Choose a reason for hiding this comment

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

I think the syntax highlighting works fine on our website 👍

{
"svelte/brackets-same-line": [
Copy link

Choose a reason for hiding this comment

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

should be svelte/html-closing-bracket-new-line?

"error",
{
"singleline": "never", // ["never", "always"]
"multiline": "always", // ["never", "always"]
"selfClosingTag": {
"singleline": "never", // ["never", "always"]
"multiline": "always" // ["never", "always"]
}
}
]
}
```

- `singleline`: (`"never"` by default) Configuration for single-line elements. It's a single-line element if the element does not have attributes or the last attribute is on the same line as the opening bracket.
- `multiline`: (`"always"` by default) Configuration for multi-line elements. It's a multi-line element if the last attribute is not on the same line of the opening bracket.
- `selfClosingTag.singleline`: Configuration for single-line self closing elements.
- `selfClosingTag.multiline`: Configuration for multi-line self closing elements.

The `selfClosing` is optional, and by default it will use the same configuration as `singleline` and `multiline`, respectively.

## :mag: Implementation

- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/src/rules/html-closing-bracket-new-line.ts)
- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/packages/eslint-plugin-svelte/tests/src/rules/html-closing-bracket-new-line.ts)
1 change: 1 addition & 0 deletions packages/eslint-plugin-svelte/src/configs/flat/prettier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const config: Linter.Config[] = [
rules: {
// eslint-plugin-svelte rules
'svelte/first-attribute-linebreak': 'off',
'svelte/html-closing-bracket-new-line': 'off',
'svelte/html-closing-bracket-spacing': 'off',
'svelte/html-quotes': 'off',
'svelte/html-self-closing': 'off',
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin-svelte/src/configs/prettier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const config: Linter.LegacyConfig = {
rules: {
// eslint-plugin-svelte rules
'svelte/first-attribute-linebreak': 'off',
'svelte/html-closing-bracket-new-line': 'off',
'svelte/html-closing-bracket-spacing': 'off',
'svelte/html-quotes': 'off',
'svelte/html-self-closing': 'off',
Expand Down
14 changes: 14 additions & 0 deletions packages/eslint-plugin-svelte/src/rule-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ export interface RuleOptions {
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/first-attribute-linebreak/
*/
'svelte/first-attribute-linebreak'?: Linter.RuleEntry<SvelteFirstAttributeLinebreak>
/**
* Require or disallow a line break before tag's closing brackets
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/html-closing-bracket-new-line/
*/
'svelte/html-closing-bracket-new-line'?: Linter.RuleEntry<SvelteHtmlClosingBracketNewLine>
/**
* require or disallow a space before tag's closing brackets
* @see https://sveltejs.github.io/eslint-plugin-svelte/rules/html-closing-bracket-spacing/
Expand Down Expand Up @@ -361,6 +366,15 @@ type SvelteFirstAttributeLinebreak = []|[{
multiline?: ("below" | "beside")
singleline?: ("below" | "beside")
}]
// ----- svelte/html-closing-bracket-new-line -----
type SvelteHtmlClosingBracketNewLine = []|[{
singleline?: ("always" | "never")
multiline?: ("always" | "never")
selfClosingTag?: {
singleline?: ("always" | "never")
multiline?: ("always" | "never")
}
}]
// ----- svelte/html-closing-bracket-spacing -----
type SvelteHtmlClosingBracketSpacing = []|[{
startTag?: ("always" | "never" | "ignore")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
import type { AST } from 'svelte-eslint-parser';
import { createRule } from '../utils';
import { getSourceCode } from '../utils/compat';
import type { SourceCode } from '../types';

type ExpectedNode = AST.SvelteStartTag | AST.SvelteEndTag;
type OptionValue = 'always' | 'never';
type RuleOptions = {
singleline: OptionValue;
multiline: OptionValue;
selfClosingTag?: Omit<RuleOptions, 'selfClosingTag'>;
};

function getPhrase(lineBreaks: number) {
switch (lineBreaks) {
case 0: {
return 'no line breaks';
}
case 1: {
return '1 line break';
}
default: {
return `${lineBreaks} line breaks`;
}
}
}

function getExpectedLineBreaks(
node: ExpectedNode,
options: RuleOptions,
type: keyof Omit<RuleOptions, 'selfClosingTag'>
) {
const isSelfClosingTag = node.type === 'SvelteStartTag' && node.selfClosing;
if (isSelfClosingTag && options.selfClosingTag && options.selfClosingTag[type]) {
return options.selfClosingTag[type] === 'always' ? 1 : 0;
}

return options[type] === 'always' ? 1 : 0;
}

type NodeData = {
actualLineBreaks: number;
expectedLineBreaks: number;
startToken: AST.Token;
endToken: AST.Token;
};

function getSelfClosingData(
sourceCode: SourceCode,
node: AST.SvelteStartTag,
options: RuleOptions
): NodeData | null {
const tokens = sourceCode.getTokens(node);
const closingToken = tokens[tokens.length - 2];
if (closingToken.value !== '/') {
return null;
}

const prevToken = sourceCode.getTokenBefore(closingToken)!;
const type = node.loc.start.line === prevToken.loc.end.line ? 'singleline' : 'multiline';

const expectedLineBreaks = getExpectedLineBreaks(node, options, type);
const actualLineBreaks = closingToken.loc.start.line - prevToken.loc.end.line;

return { actualLineBreaks, expectedLineBreaks, startToken: prevToken, endToken: closingToken };
}

function getNodeData(
sourceCode: SourceCode,
node: ExpectedNode,
options: RuleOptions
): NodeData | null {
const closingToken = sourceCode.getLastToken(node);
if (closingToken.value !== '>') {
return null;
}

const prevToken = sourceCode.getTokenBefore(closingToken)!;
const type = node.loc.start.line === prevToken.loc.end.line ? 'singleline' : 'multiline';

const expectedLineBreaks = getExpectedLineBreaks(node, options, type);
const actualLineBreaks = closingToken.loc.start.line - prevToken.loc.end.line;

return { actualLineBreaks, expectedLineBreaks, startToken: prevToken, endToken: closingToken };
}

export default createRule('html-closing-bracket-new-line', {
meta: {
docs: {
description: "Require or disallow a line break before tag's closing brackets",
category: 'Stylistic Issues',
recommended: false,
conflictWithPrettier: true
},
schema: [
{
type: 'object',
properties: {
singleline: { enum: ['always', 'never'] },
multiline: { enum: ['always', 'never'] },
selfClosingTag: {
type: 'object',
properties: {
singleline: { enum: ['always', 'never'] },
multiline: { enum: ['always', 'never'] }
},
additionalProperties: false,
minProperties: 1
}
},
additionalProperties: false
}
],
messages: {
expectedBeforeClosingBracket:
'Expected {{expected}} before closing bracket, but {{actual}} found.'
},
fixable: 'code',
type: 'suggestion'
},
create(context) {
const options: RuleOptions = context.options[0] ?? {};
options.singleline ??= 'never';
options.multiline ??= 'always';

const sourceCode = getSourceCode(context);

return {
'SvelteStartTag, SvelteEndTag'(node: ExpectedNode) {
const data =
node.type === 'SvelteStartTag' && node.selfClosing
? getSelfClosingData(sourceCode, node, options)
: getNodeData(sourceCode, node, options);
if (!data) {
return;
}

const { actualLineBreaks, expectedLineBreaks, startToken, endToken } = data;
if (actualLineBreaks !== expectedLineBreaks) {
// For SvelteEndTag, does not make sense to add a line break, so we only fix if there are extra line breaks
if (node.type === 'SvelteEndTag' && expectedLineBreaks !== 0) {
return;
}

context.report({
node,
loc: { start: startToken.loc.end, end: endToken.loc.start },
messageId: 'expectedBeforeClosingBracket',
data: {
expected: getPhrase(expectedLineBreaks),
actual: getPhrase(actualLineBreaks)
},
fix(fixer) {
const range: AST.Range = [startToken.range[1], endToken.range[0]];
const text = '\n'.repeat(expectedLineBreaks);
return fixer.replaceTextRange(range, text);
}
});
}
}
};
}
});
2 changes: 2 additions & 0 deletions packages/eslint-plugin-svelte/src/utils/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import derivedHasSameInputsOutputs from '../rules/derived-has-same-inputs-output
import experimentalRequireSlotTypes from '../rules/experimental-require-slot-types';
import experimentalRequireStrictEvents from '../rules/experimental-require-strict-events';
import firstAttributeLinebreak from '../rules/first-attribute-linebreak';
import htmlClosingBracketNewLine from '../rules/html-closing-bracket-new-line';
import htmlClosingBracketSpacing from '../rules/html-closing-bracket-spacing';
import htmlQuotes from '../rules/html-quotes';
import htmlSelfClosing from '../rules/html-self-closing';
Expand Down Expand Up @@ -75,6 +76,7 @@ export const rules = [
experimentalRequireSlotTypes,
experimentalRequireStrictEvents,
firstAttributeLinebreak,
htmlClosingBracketNewLine,
htmlClosingBracketSpacing,
htmlQuotes,
htmlSelfClosing,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"options": [{ "multiline": "never" }]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
- message: Expected no line breaks before closing bracket, but 1 line break found.
line: 2
column: 12
suggestions: null
- message: Expected no line breaks before closing bracket, but 1 line break found.
line: 7
column: 12
suggestions: null
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<div
class="foo"
></div>
<div
class="bar"></div>
<div
class="bar"
>
Children
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<div
class="foo"></div>
<div
class="bar"></div>
<div
class="bar">
Children
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"options": [{ "selfClosingTag": { "singleline": "always", "multiline": "always" } }]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
- message: Expected 1 line break before closing bracket, but no line breaks found.
line: 1
column: 18
suggestions: null
- message: Expected 1 line break before closing bracket, but 2 line breaks found.
line: 6
column: 12
suggestions: null
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Custom foo="bar" />
<Custom
foo="bar"
/>
<Custom
foo="bar"

/>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Custom foo="bar"
/>
<Custom
foo="bar"
/>
<Custom
foo="bar"
/>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"options": [{ "selfClosingTag": { "singleline": "never", "multiline": "never" } }]
}
Loading
Loading