8000 [sort-ngmodule-metadata-arrays] `--fix` not able to correct all the reports once · Issue #675 · angular-eslint/angular-eslint · GitHub
[go: up one dir, main page]
Skip to content

[sort-ngmodule-metadata-arrays] --fix not able to correct all the reports once #675

New iss 8000 ue

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

Sign up for GitHub

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

Jump to bottom
Closed
splincode opened this issue Sep 9, 2021 · 5 comments
Closed

[sort-ngmodule-metadata-arrays] --fix not able to correct all the reports once #675

splincode opened this issue Sep 9, 2021 · 5 comments
Assignees
@rafaelss95
Labels
bug Something isn't working package: eslint-plugin-template Angular Template rules

Comments

@splincode
Copy link
splincode commented Sep 9, 2021 •
edited by rafaelss95
Loading 
{
  "rules": {
    "@angular-eslint/sort-ngmodule-metadata-arrays": "error"
  }
}

After run with --fix didn't work for my cases:

// ...

@NgModule({
    imports: [
        CommonModule,
        DeepPathPipeModule,
        DefaultValuePipeModule,
        DragDropModule,
        MatIconModule,
        MergeCssClassesPipeModule,
        SafePipeModule
    ],
    declarations: [
        AutoHeightDirective,
        NgxColumnComponent,
        NgxOptionsComponent,
        TableBuilderComponent,
        TableTbodyComponent,
        TableTheadComponent,
        TableCellComponent,
        TemplateBodyTdDirective,
        TemplateHeadThDirective,
        ObserverViewDirective,
        NgxContextMenuComponent,
        NgxContextMenuItemComponent,
        NgxContextMenuDividerComponent,
        NgxMenuContentComponent,
        NgxEmptyComponent,
        NgxHeaderComponent,
        NgxFooterComponent,
        NgxFilterViewerComponent,
        NgxFilterComponent,
        NgxFilterDirective,
        DragIconComponent,
        NgxSourceNullComponent,
        DisableRowPipe,
        TableSelectedItemsPipe,
        MapToTableEntriesPipe,
        VirtualForDirective,
        GetFreeSizePipe,
        GetClientHeightPipe
    ],
    providers: [WebWorkerThreadService],
    exports: [
        NgxColumnComponent,
        NgxOptionsComponent,
        NgxContextMenuComponent,
        TableBuilderComponent,
        TemplateBodyTdDirective,
        TemplateHeadThDirective,
        NgxContextMenuItemComponent,
        NgxContextMenuDividerComponent,
        NgxMenuContentComponent,
        NgxEmptyComponent,
        NgxHeaderComponent,
        NgxFooterComponent,
        NgxFilterViewerComponent,
        NgxFilterComponent,
        NgxFilterDirective,
        TableSelectedItemsPipe,
        MapToTableEntriesPipe,
        NgxSourceNullComponent
    ]
})
// ...

Versions

package version
@angular-eslint/eslint-plugin 12.3.1
@typescript-eslint/parser 4.31.0
ESLint 7.32.0
node 16.0
@splincode splincode added package: eslint-plugin-template Angular Template rules triage This issue needs to be looked at and categorized by a maintainer labels Sep 9, 2021
@splincode splincode changed the title [rulename] issue title [@angular-eslint/template/sort-ngmodule-metadata-arrays] autofix not working Sep 9, 2021
@splincode
Copy link
Author
splincode commented Sep 9, 2021 •
edited
Loading

also

// ..

@NgModule({
    declarations: [AppComponent],
    imports: [
        AppRoutingModule,
        BrowserModule,
        FlexLayoutModule,
        HttpClientModule,
        MatListModule,
        MatMenuModule,
        MatSidenavModule,
        MatToolbarModule,
        BrowserAnimationsModule
    ],
    bootstrap: [AppComponent]
})
// ...

and not sorted

// ...

@NgModule({
    imports: [
        TableBuilderModule.forRoot(),
        CommonModule,
        FormsModule,
        MatButtonModule,
        MatInputModule,
        MatMenuModule,
        ReactiveFormsModule
    ],
    entryComponents: [CodeDialogComponent, DialogTemplateComponent],
    declarations: [CodeDialogComponent, DialogTemplateComponent],
    exports: [
        CodeDialogComponent,
        DragDropModule,
        FormsModule,
        MatButtonModule,
        MatCardModule,
        MatCheckboxModule,
        MatDialogModule,
        MatDividerModule,
        MatInputModule,
        MatListModule,
        MatProgressSpinnerModule,
        MatSelectModule,
        MatIconModule,
        MatSidenavModule,
        MatSnackBarModule,
        MatToolbarModule,
        ScrollingModule,
        TableBuilderModule,
        MatTableModule,
        MatTabsModule,
        MatMenuModule
    ]
})
// ...

@splincode
Copy link
Author

@timdeschryver could you help me?

@rafaelss95
Copy link
Member
rafaelss95 commented Sep 9, 2021 •
edited
Loading

Hi @splincode, I'm not Tim, but I'll try to help you 🖖

The fixer itself indeed works as you can see here (and in many other places in the file mentioned below), for example:

angular-eslint/packages/eslint-plugin/tests/rules/sort-ngmodule-metadata-arrays/cases.ts

Lines 82 to 99 in b38d828

convertAnnotatedSourceToFailureCase({
description: 'should fail if `imports` metadata arrays is not sorted ASC',
annotatedSource: `
@NgModule({
imports: [aModule, bModule, DModule, cModule]
~~~~~~~
})
class Test {}
`,
messageId,
annotatedOutput: `
@NgModule({
imports: [aModule, bModule, cModule, DModule]
~~~~~~~
})
class Test {}
`,
}),

The problem you might be facing is that, as described at the official ESLint documentation, the fixer process will repeat up to 10 times, or until no more fixable problems are found. In other words, to completely fix the code you showed us above, according to this rule, you would probably need more than 10 iterations/processes and that's why you still see report problems.

On the other hand, I can see that the algorithm that finds unordered nodes can be improved to avoid as many iterations as currently:

angular-eslint/packages/eslint-plugin/src/rules/sort-ngmodule-metadata-arrays.ts

Lines 36 to 41 in f993069

const unorderedNodes = elements
.filter(ASTUtils.isIdentifier)
.map((current, index, list) => [current, list[index + 1]])
.find(([current, next]) => {
return next && current.name.localeCompare(next.name) === 1;
});

But note that, depending on how many changes are needed, you'll still hit the 10-limit problem.

Also, minor notes about your issue report:

  • Posting codes (the minimum possible to reproduce the problem, in this case, only the @NgModule part is necessary) is preferable to screenshots (not both, please) are better because besides being able to copy, it reduces the amount of space occupied;
  • Use the real version instead of latest in the versions sections, as this can help us both now and in the future by looking for past issues and seeing which version was used at the time and then identifying a possible regression :)

@rafaelss95 rafaelss95 removed the triage This issue needs to be looked at and categorized by a maintainer label Sep 9, 2021
@splincode
Copy link
Author

But note that, depending on how many changes are needed, you'll still hit the 10-limit problem.
https://github.com/lydell/eslint-plugin-simple-import-sort
Yes, but rule import sort works without 10 iterations, I expect the same behavior ))

On the other hand, I can see that the algorithm that finds unordered nodes can be improved to avoid as many iterations as currently

Can you improved this code?

Use the real version instead of latest fixed

@rafaelss95 rafaelss95 changed the title [@angular-eslint/template/sort-ngmodule-metadata-arrays] autofix not working [sort-ngmodule-metadata-arrays] --fix not able to correct all the reports once Sep 9, 2021
@rafaelss95 rafaelss95 added the enhancement New feature or request label Sep 9, 2021
@rafaelss95 rafaelss95 self-assigned this Sep 9, 2021
rafaelss95 mentioned this issue Sep 10, 2021
Closed
@rafaelss95 rafaelss95 mentioned this issue Sep 20, 2021
Closed
@rafaelss95 rafaelss95 added bug Something isn't working and removed enhancement New feature or request labels Sep 23, 2021
This was referenced Nov 24, 2022
Closed
Closed
@JamesHenry
Copy link
Member

Thanks all, closing in favour of #1232 please subscribe to that issue for updates

@JamesHenry JamesHenry closed this as not planned Won't fix, can't repro, duplicate, stale Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rafaelss95 rafaelss95

Labels
bug Something isn't working package: eslint-plugin-template Angular Template rules
Projects
None yet
Milestone
No milestone
3 participants
@JamesHenry @rafaelss95 @splincode
0