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

Lint using putout part 3 #3269

Merged
merged 1 commit into from
Mar 29, 2021
Merged

Lint using putout part 3 #3269

merged 1 commit into from
Mar 29, 2021

Conversation

coderaiser
Copy link
Contributor

@coderaiser coderaiser commented Mar 28, 2021

After successfully merged linting sessions part1 and part2, the time is come for part 3 :).

Any rule can be disabled.
Command used:

putout {src,addons}/**/*.ts --fix

Applied rules:

Current config for putout v16:

{
    "rules": {
        "strict-mode": "off",
        "remove-useless-spread/object": "off",
        "remove-iife": "off",
        "convert-generic-to-shorthand": "off",
        "convert-assignment-to-comparison": "off",
        "remove-console": "off",
        "remove-unused-variables": "off",
        "remove-useless-variables": "off",
        "apply-destructuring": "off",
        "merge-if-statements": "off",
        "convert-apply-to-spread": "off",
        "convert-math-pow": "off",
        "convert-for-to-for-of": "off",
        "convert-template-to-string": "off",
        "remove-empty": "on",
        "convert-index-of-to-includes": "on",
        "convert-for-each-to-for-of": "on",
        "apply-shorthand-properties": "on",
        "apply-numeric-separators": "off",
        "promises/add-missing-await": "off"
    },
    "match": {
        "SerializeAddon.benchmark.ts": {
            "remove-unused-expressions": "off"
        }
    },
    "plugins": [
        "apply-shorthand-properties"
    ]
}

Copy link
Member

@jerch jerch left a comment

Choose a reason for hiding this comment

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

Great work, your tools seems to be quite capable 👍 . Nothing to mock about from my side.

Edit:
Ok lemme re-phrase that - your tool seems to make a really great job in transpiling and finding code smells/errors. Maybe you should think about promoting it to other tools like linters/babel, so they can directly benefit from its capabilities?

@@ -45,7 +45,7 @@ describe('FitAddon', () => {

describe('proposeDimensions', () => {
afterEach(async () => {
return unloadFit();
return await unloadFit();
Copy link
Member

Choose a reason for hiding this comment

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

Wow - did your tool find that on its own? Just wondering, because not awaiting a promise might create bigger havoc and normally would qualify as a coding error (though not sure if thats the case here, did not look up the code).
If your tool is capable to find those things it has true benefits. 👍

Copy link
Contributor Author

@coderaiser coderaiser Mar 29, 2021

Choose a reason for hiding this comment

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

Yes :), the rule is promises/add-return-await, what it does is looking for declarations of functions and if it is async, it adds await.

Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Looks good, love the for ... of changes as I've come to dislike forEach greatly 🙂

@Tyriar Tyriar added this to the 4.12.0 milestone Mar 29, 2021
@Tyriar Tyriar merged commit e78561b into xtermjs:master Mar 29, 2021
@coderaiser coderaiser deleted the lint-using-putout branch March 29, 2021 10:13
@coderaiser
Copy link
Contributor Author

Great work, your tools seems to be quite capable 👍 . Nothing to mock about from my side.

Thank you :), I'm using it a lot for all my projects, and want it to be useful for community as much as possible :).

Edit:
Ok lemme re-phrase that - your tool seems to make a really great job in transpiling and finding code smells/errors. Maybe you should think about promoting it to other tools like linters/babel, so they can directly benefit from its capabilities?

putout uses babel from the inside, and it benefit a lot from babel and @babel/template and move code transforming to the next level, using custom language fully compatible with JavaScript with help of @putout/compare, like in remove-useless-conditions:

module.exports.replace = () => ({
    'if (__a?.__b) {__a.__b(__args)}': '__a?.__b(__args)',
});

Also it calls eslint after transforming is done.

So, because eslint uses estree AST, they unlikely will use babel tooling which uses babel AST.
Guys from rome writes everything from scratch, and using same visitor traversing which is good for speed, good for speed but challenging for support.

Also putout has a progress bar, and support for md, yaml, html, css, json files from the box, because integrations with other tooling is top priority.

Anyways for babel it can be useful, because of simplified transformations of the code, so this is a good idea and I will be glad for any help :). From the first day putout written as Open Source solution divided on independent packages to make code transforming simple, popular and available to everyone.

Looks good, love the for ... of changes as I've come to dislike forEach greatly

Nice :), same with me, for-of looks much cleaner

@Tyriar
Copy link
Member

Tyriar commented Mar 29, 2021

Nice :), same with me, for-of looks much cleaner

The main reason for me is debugging forEach is a pain since it uses callback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants