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.

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

Bug: Fix for sort-decorators moves decorators above JSDoc comments #399

Open
2 tasks done
reduckted opened this issue Nov 27, 2024 · 2 comments · May be fixed by #402
Open
2 tasks done

Bug: Fix for sort-decorators moves decorators above JSDoc comments #399

reduckted opened this issue Nov 27, 2024 · 2 comments · May be fixed by #402
Labels
bug Something isn't working

Comments

@reduckted
Copy link

Describe the bug

The fix for the sort-decorators rule will move a decorator above the JSDoc comment.

It seems as though the JSDoc comment is being kept with the directive that immediately follows it, which makes sense for normal comments, but I would expect JSDoc comments to stay before all directives.

ESLint config:

const perfectionist = require('eslint-plugin-perfectionist');
const { config, configs } = require('typescript-eslint');

module.exports = config(configs.recommended, {
    plugins: { perfectionist },
    rules: {
        'perfectionist/sort-decorators': 'warn',
    },
});

Original code:

export class Test {
    /**
     * Test.
     */
    @B()
    @A()
    foo: number;
}

Actual fixed code:

export class Test {
    @A()
    /**
     * Test.
     */
    @B()
    foo: number;
}

Expected fixed code:

export class Test {
    /**
     * Test.
     */
    @A()
    @B()
    foo: number;
}

Code example

export class Test {
    /**
     * Test.
     */
    @B()
    @A()
    foo: number;
}

ESLint version

v9.15.0

ESLint Plugin Perfectionist version

v4.1.2

Additional comments

No response

Validations

  • Read the docs.
  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
@reduckted reduckted added the bug Something isn't working label Nov 27, 2024
@hugop95
Copy link
Contributor

hugop95 commented Nov 27, 2024 •
edited
Loading

Hi @reduckted ,
I understand the concern even though I think it's a bit context-dependant.

Theoretically, making top-level comments stay where they are should fix the issue. The drawback is that a decorator comment that gets put to the top will now be fixed unless there is already a top-level comment in place.

I'm not sure yet if this would be better to put that behaviour behind an option or not. I suppose that your use-case is probably the most common one among all users.

@reduckted
Copy link
Author

It's not so much that it's a top-level comment. It's that it's a JSDoc comment. I can't think of any reason why a decorator should appear above a JSDoc comment.

What makes this scenario worse is when you also use the eslint-plugin-jsdoc plugin with the require-jsdoc rule enabled. There's a fixer for that rule that adds an empty JsDoc comment. What ends up happening is the sort-decorators fixer moves the decorator above the JSDoc comment, then the require-jsdoc fixer sees that there isn't a JSDoc comment above the decorators, so it adds a blank one. You end up with this mess:

export class Test {
    /**
     *
     */
    @A()
    /**
     * Test.
     */
    @B()
    foo: number;
}

@hugop95 hugop95 linked a pull request Nov 28, 2024 that will close this issue
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(sort-decorators): keeps JSDoc blocks in place hugop95/eslint-plugin-perfectionist
2 participants
@reduckted @hugop95