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: adds customGroup.newlinesInside option #428

Merged
merged 13 commits into from
Dec 23, 2024
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
3 changes: 3 additions & 0 deletions docs/content/rules/sort-classes.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,7 @@ interface CustomGroupDefinition {
groupName: string
type?: 'alphabetical' | 'natural' | 'line-length' | 'unsorted'
order?: 'asc' | 'desc'
newlinesInside?: 'always' | 'never'
selector?: string
modifiers?: string[]
elementNamePattern?: string
Expand All @@ -615,6 +616,7 @@ interface CustomGroupAnyOfDefinition {
groupName: string
type?: 'alphabetical' | 'natural' | 'line-length' | 'unsorted'
order?: 'asc' | 'desc'
newlinesInside?: 'always' | 'never'
anyOf: Array<{
selector?: string
modifiers?: string[]
Expand All @@ -637,6 +639,7 @@ A class member will match a `CustomGroupAnyOfDefinition` group if it matches all
- `decoratorNamePattern`: If entered, will check that at least one `decorator` matches the pattern entered.
- `type`: Overrides the sort type for that custom group. `unsorted` will not sort the group.
- `order`: Overrides the sort order for that custom group
- `newlinesInside`: Enforces a specific newline behavior between elements of the group.

#### Match importance

Expand Down
1 change: 1 addition & 0 deletions docs/content/rules/sort-interfaces.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,7 @@ An interface member will match a `CustomGroupAnyOfDefinition` group if it matche
- `elementNamePattern`: If entered, will check that the name of the element matches the pattern entered.
- `type`: Overrides the sort type for that custom group. `unsorted` will not sort the group.
- `order`: Overrides the sort order for that custom group
- `newlinesInside`: Enforces a specific newline behavior between elements of the group.

#### Match importance

Expand Down
1 change: 1 addition & 0 deletions docs/content/rules/sort-modules.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ A module member will match a `CustomGroupAnyOfDefinition` group if it matches al
- `decoratorNamePattern`: If entered, will check that at least one `decorator` matches the pattern entered.
- `type`: Overrides the sort type for that custom group. `unsorted` will not sort the group.
- `order`: Overrides the sort order for that custom group
- `newlinesInside`: Enforces a specific newline behavior between elements of the group.

#### Match importance

Expand Down
1 change: 1 addition & 0 deletions docs/content/rules/sort-object-types.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,7 @@ An object type will match a `CustomGroupAnyOfDefinition` group if it matches all
- `elementNamePattern`: If entered, will check that the name of the element matches the pattern entered.
- `type`: Overrides the sort type for that custom group. `unsorted` will not sort the group.
- `order`: Overrides the sort order for that custom group
- `newlinesInside`: Enforces a specific newline behavior between elements of the group.

#### Match importance

Expand Down
26 changes: 13 additions & 13 deletions rules/sort-classes/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,19 @@ export type SingleCustomGroup =
| BaseSingleCustomGroup<ConstructorSelector>
| AdvancedSingleCustomGroup<MethodSelector>

export type CustomGroup = (
| {
order?: SortClassesOptions[0]['order']
type?: SortClassesOptions[0]['type']
}
| {
type?: 'unsorted'
}
) & {
newlinesInside?: 'always' | 'never'
groupName: string
} & (SingleCustomGroup | AnyOfCustomGroup)

export type NonDeclarePropertyGroup = JoinWithDash<
[
PublicOrProtectedOrPrivateModifier,
Expand Down Expand Up @@ -202,19 +215,6 @@ type Group =
| 'unknown'
| string

type CustomGroup = (
| {
order?: SortClassesOptions[0]['order']
type?: SortClassesOptions[0]['type']
}
| {
type?: 'unsorted'
}
) &
(SingleCustomGroup | AnyOfCustomGroup) & {
groupName: string
}

type AdvancedSingleCustomGroup<T extends Selector> = {
decoratorNamePattern?: string
elementValuePattern?: string
Expand Down
10 changes: 8 additions & 2 deletions rules/sort-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -537,12 +537,15 @@ export default createEslintRule<Options<string[]>, MESSAGE_ID>({
messageIds = [
...messageIds,
...getNewlinesErrors({
options: {
...options,
customGroups: [],
},
missedSpacingError: 'missedSpacingBetweenImports',
extraSpacingError: 'extraSpacingBetweenImports',
rightNum: rightNumber,
leftNum: leftNumber,
sourceCode,
options,
right,
left,
}),
Expand All @@ -559,10 +562,13 @@ export default createEslintRule<Options<string[]>, MESSAGE_ID>({
fixer,
}),
...makeNewlinesFixes({
options: {
...options,
customGroups: [],
},
sortedNodes: sortedNodesExcludingEslintDisabled,
nodes: nodeList,
sourceCode,
options,
fixer,
}),
],
Expand Down
8 changes: 4 additions & 4 deletions rules/sort-modules/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,10 @@ type CustomGroup = (
| {
type?: 'unsorted'
}
) &
(SingleCustomGroup | AnyOfCustomGroup) & {
groupName: string
}
) & {
newlinesInside?: 'always' | 'never'
groupName: string
} & (SingleCustomGroup | AnyOfCustomGroup)
/**
* Only used in code, so I don't know if it's worth maintaining this.
*/
Expand Down
8 changes: 4 additions & 4 deletions rules/sort-object-types/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,10 @@ type CustomGroup = (
| {
type?: 'unsorted'
}
) &
(SingleCustomGroup | AnyOfCustomGroup) & {
groupName: string
}
) & {
newlinesInside?: 'always' | 'never'
groupName: string
} & (SingleCustomGroup | AnyOfCustomGroup)

type IndexSignatureGroup = JoinWithDash<
[
Expand Down
108 changes: 108 additions & 0 deletions test/rules/sort-classes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7787,6 +7787,114 @@ describe(ruleName, () => {
invalid: [],
},
)

describe('newlinesInside', () => {
ruleTester.run(
`${ruleName}: allows to use newlinesInside: always`,
rule,
{
invalid: [
{
errors: [
{
data: {
right: 'c',
left: 'b',
},
messageId: 'missedSpacingBetweenClassMembers',
},
{
data: {
right: 'd',
left: 'c',
},
messageId: 'extraSpacingBetweenClassMembers',
},
],
options: [
{
customGroups: [
{
groupName: 'methodsWithNewlinesInside',
newlinesInside: 'always',
selector: 'method',
},
],
groups: ['unknown', 'methodsWithNewlinesInside'],
},
],
code: dedent`
class Class {
a
b() {}
c() {}


d() {}
}
`,
output: dedent`
class Class {
a
b() {}

c() {}

d() {}
}
`,
},
],
valid: [],
},
)

ruleTester.run(`${ruleName}: allows to use newlinesInside: never`, rule, {
invalid: [
{
options: [
{
customGroups: [
{
groupName: 'methodsWithoutNewlinesInside',
newlinesInside: 'never',
selector: 'method',
},
],
groups: ['unknown', 'methodsWithoutNewlinesInside'],
},
],
errors: [
{
data: {
right: 'd',
left: 'c',
},
messageId: 'extraSpacingBetweenClassMembers',
},
],
output: dedent`
class Class {
a

c() {}
d() {}
}
`,
code: dedent`
class Class {
a

c() {}

d() {}
}
`,
},
],
valid: [],
})
})
})

describe(`${ruleName}: misc`, () => {
Expand Down
63 changes: 0 additions & 63 deletions test/rules/sort-imports.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,13 +290,6 @@ describe(ruleName, () => {
},
messageId: 'unexpectedImportsGroupOrder',
},
{
Copy link
Owner

Choose a reason for hiding this comment

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

Can you tell me why you removed these tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@azat-io

  • The edited code responsible for those lines changes can be found here (see each commit's description)
  • Those changes are not absolutely required:
    • (1) Some spacing errors were reported after checking if leftNum < rightNum.
    • (2) Some other errors were not checking if leftNum < rightNum, leading to spacing errors being reported even when the nodes were not sorted.
    • I added the leftNum < rightNum for all cases => Spacing errors from (2) are now not reported.
  • If needed, I can revert these commits. 🙂

data: {
left: '~/c',
right: 't',
},
messageId: 'extraSpacingBetweenImports',
},
{
data: {
rightGroup: 'internal',
Expand All @@ -306,13 +299,6 @@ describe(ruleName, () => {
},
messageId: 'unexpectedImportsGroupOrder',
},
{
data: {
left: '../../e',
right: '~/b',
},
messageId: 'extraSpacingBetweenImports',
},
],
options: [
{
Expand Down Expand Up @@ -899,13 +885,6 @@ describe(ruleName, () => {
},
messageId: 'unexpectedImportsGroupOrder',
},
{
data: {
left: './b',
right: 'c',
},
messageId: 'extraSpacingBetweenImports',
},
],
options: [
{
Expand Down Expand Up @@ -2630,13 +2609,6 @@ describe(ruleName, () => {
},
messageId: 'unexpectedImportsGroupOrder',
},
{
data: {
left: '~/c',
right: 't',
},
messageId: 'extraSpacingBetweenImports',
},
{
data: {
rightGroup: 'internal',
Expand All @@ -2646,13 +2618,6 @@ describe(ruleName, () => {
},
messageId: 'unexpectedImportsGroupOrder',
},
{
data: {
left: '../../e',
right: '~/b',
},
messageId: 'extraSpacingBetweenImports',
},
],
options: [
{
Expand Down Expand Up @@ -3239,13 +3204,6 @@ describe(ruleName, () => {
},
messageId: 'unexpectedImportsGroupOrder',
},
{
data: {
left: './b',
right: 'c',
},
messageId: 'extraSpacingBetweenImports',
},
],
options: [
{
Expand Down Expand Up @@ -4251,13 +4209,6 @@ describe(ruleName, () => {
},
messageId: 'unexpectedImportsGroupOrder',
},
{
data: {
left: '~/c',
right: 't',
},
messageId: 'extraSpacingBetweenImports',
},
{
data: {
rightGroup: 'internal',
Expand All @@ -4267,13 +4218,6 @@ describe(ruleName, () => {
},
messageId: 'unexpectedImportsGroupOrder',
},
{
data: {
left: '../../e',
right: '~/b',
},
messageId: 'extraSpacingBetweenImports',
},
],
options: [
{
Expand Down Expand Up @@ -4830,13 +4774,6 @@ describe(ruleName, () => {
},
messageId: 'unexpectedImportsGroupOrder',
},
{
data: {
left: './b',
right: 'c',
},
messageId: 'extraSpacingBetweenImports',
},
],
options: [
{
Expand Down
Loading
Loading