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

Halloween Lint using Putout (part 5) #3538

Merged
merged 3 commits into from
Nov 8, 2021

Conversation

coderaiser
Copy link
Contributor

@coderaiser coderaiser commented Nov 2, 2021

After successfully merged linting sessions part 1, part 2 and part 3, part 4 the time is come for part 5 :).

As usual, any rule can be disabled.

Command used:

putout . --fix

Applied rules:

Current config for putout v21:

{
    "rules": {
        "apply-is-array": ["on", {
            "inline": true
        },
        "remove-useless-new": "off",
        "remove-useless-return": "off",
        "remove-useless-types-from-constants": "off",
        "remove-useless-type-convertion/with-double-negations": "off",
        "convert-typeof-to-is-type": "off",
        "apply-shorthand-properties": "off",
        "apply-destructuring": "off",
        "apply-numeric-separators": "off",
        "convert-assignment-to-comparison": "off",
        "convert-apply-to-spread": "off",
        "convert-math-pow": "off",
        "convert-for-to-for-of": "off",
        "convert-template-to-string": "off",
        "strict-mode": "off",
        "remove-useless-spread/object": "off",
        "remove-useless-array-constructor": "off",
        "remove-boolean-from-assertions": "off",
        "remove-iife": "off",
        "remove-console": "off",
        "remove-unused-variables": "off",
        "remove-useless-variables": "off",
        "merge-if-statements": "off",
        "promises/add-missing-await": "off",
        "promises/remove-useless-async": "off",
        "simplify-ternary": "off"
    },
    "match": {
        "*.benchmark.ts": {
            "remove-unused-expressions": "off"
        }
    },
    "plugins": [
        "apply-shorthand-properties"
    ],
    "ignore": [
        "*.md",
        "*.json",
        "*ignore",
        "*.yml",
        ".npmrc",
        "*.css",
        "out",
        "out-test",
        "demo"
    ]
}

Happy Halloween 🎃!

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 like this caused tsc strict errors:

src/common/InputHandler.ts(64,18): error TS7006: Parameter 'a' implicitly has an 'any' type.
src/common/InputHandler.ts(561,45): error TS2339: Property 'substring' does not exist on type 'string | Uint8Array'.
  Property 'substring' does not exist on type 'Uint8Array'.
src/common/InputHandler.ts(562,43): error TS2339: Property 'subarray' does not exist on type 'string | Uint8Array'.
  Property 'subarray' does not exist on type 'string'.
src/common/input/WriteBuffer.test.ts(12,18): error TS7006: Parameter 'a' implicitly has an 'any' type.
src/common/input/WriteBuffer.test.ts(49,75): error TS2345: Argument of type 'string | Uint8Array' is not assignable to parameter of type 'Uint8Array'.
  Type 'string' is not assignable to type 'Uint8Array'.
src/common/parser/EscapeSequenceParser.test.ts(17,18): error TS7006: Parameter 'a' implicitly has an 'any' type.
src/common/parser/EscapeSequenceParser.test.ts(1737,7): error TS2322: Type 'unknown' is not assignable to type 'string | undefined'.
  Type 'unknown' is not assignable to type 'string'.
addons/xterm-addon-attach/src/AttachAddon.ts(10,18): error TS7006: Parameter 'a' implicitly has an 'any' type.
addons/xterm-addon-attach/src/AttachAddon.ts(32,24): error TS2345: Argument of type 'string | ArrayBuffer' is not assignable to parameter of type 'string | Uint8Array'.
  Type 'ArrayBuffer' is not assignable to type 'string | Uint8Array'.
    Type 'ArrayBuffer' is missing the following properties from type 'Uint8Array': BYTES_PER_ELEMENT, buffer, byteOffset, copyWithin, and 22 more.
addons/xterm-addon-attach/src/AttachAddon.ts(32,63): error TS2769: No overload matches this call.
  The last overload gave the following error.
    Argument of type 'string | ArrayBuffer' is not assignable to parameter of type 'ArrayBuffer'.
      Type 'string' is not assignable to type 'ArrayBuffer'.
error TS2688: Cannot find type definition file for '../../../out-test/api/TestUtils'.
  The file is in the program because:
    Entry point of type library '../../../out-test/api/TestUtils' specified in compilerOptions
error TS2688: Cannot find type definition file for '../../../out-test/api/TestUtils'.
  The file is in the program because:
    Entry point of type library '../../../out-test/api/TestUtils' specified in compilerOptions
error TS2688: Cannot find type definition file for '../../../out-test/api/TestUtils'.
  The file is in the program because:
    Entry point of type library '../../../out-test/api/TestUtils' specified in compilerOptions
error TS2688: Cannot find type definition file for '../../../out-test/api/TestUtils'.
  The file is in the program because:
    Entry point of type library '../../../out-test/api/TestUtils' specified in compilerOptions

@coderaiser
Copy link
Contributor Author

coderaiser commented Nov 4, 2021

@jerch
Copy link
Member

jerch commented Nov 4, 2021

@coderaiser

On a sidenote: most promises removed from tests, are actually needed to force the return of a promise testing promise handling in the parser. So they should not be removed there.

@coderaiser
Copy link
Contributor Author

@jerch just got back promises

@coderaiser
Copy link
Contributor Author

coderaiser commented Nov 5, 2021

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.

Lots of good changes here, I especially like the optional chaining.

Comment on lines 61 to 63
new ThroughputRuntimeCase('', () => (({
payloadSize: serializeAddon.serialize().length
})), { fork: false }).showAverageThroughput();
Copy link
Member

Choose a reason for hiding this comment

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

This was more readable before imo, also it doubled up the params for some reasons.

@@ -41,7 +41,7 @@ export default async function load(fontFamily: string, cacheSize: number): Promi
name: 'local-fonts'
});
if (status && status.state !== 'granted') {
throw new Error('Permission to access local fonts not granted.');
throw Error('Permission to access local fonts not granted.');
Copy link
Member

Choose a reason for hiding this comment

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

I kind of prefer new Error and new Array, let's remove that rule as well so this PR becomes a lot smaller. This is something a compiler should remove if they are equivalent imo.

@@ -52,7 +52,7 @@ export class CircularList<T> implements ICircularList<T> {

// Reconstruct array, starting at index 0. Only transfer values from the
// indexes 0 to length.
const newArray = new Array<T | undefined>(newMaxLength);
const newArray = Array(newMaxLength);
Copy link
Member

Choose a reason for hiding this comment

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

Not sure this was a safe thing to do removing the type here?

src/common/parser/Params.ts Outdated Show resolved Hide resolved
@coderaiser
Copy link
Contributor Author

Disabled:

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.

Thanks, always appreciated 🙂

@Tyriar Tyriar modified the milestones: 4.15.0, 4.16.0 Nov 8, 2021
@Tyriar Tyriar merged commit cb3dd19 into xtermjs:master Nov 8, 2021
@coderaiser coderaiser deleted the chore/lint-using-putout branch November 8, 2021 15:49
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