palantir / tslint-react

:orange_book: Lint rules related to React & JSX for TSLint.
Apache License 2.0
749 stars 76 forks source link

Automatically ignore null keyword usage within getDerivedStateFromProps method #244

Closed SaeedZhiany closed 5 years ago

SaeedZhiany commented 5 years ago

Bug Report

TypeScript code being linted

public static getDerivedStateFromProps(nextProps: Props): Partial<State> | null {
    if(some conditions) {
        return {
            // some state values
        };
    } else {
        return null; // <--- I got tslint error here
    }
  }

with tslint.json configuration:

{
    "extends": ["tslint-react"],
    "rules": {
        "adjacent-overload-signatures": true,
        "align": [true, "parameters", "arguments", "statements"],
        "arrow-return-shorthand": true,
        "await-promise": true,
        "ban": false,
        "ban-comma-operator": true,
        "class-name": true,
        "comment-format": [true, "check-space"],
        "curly": true,
        "deprecation": true,
        "encoding": true,
        "eofline": true,
        "forin": true,
        "import-spacing": true,
        "indent": [true, "spaces"],
        "interface-name": [true, "never-prefix"],
        "interface-over-type-literal": true,
        "jsdoc-format": true,
        "jsx-no-lambda": false,
        "jsx-no-multiline-js": false,
        "label-position": true,
        "linebreak-style": [true, "LF"],
        "max-line-length": [true, 120],
        "member-access": true,
        "member-ordering": [
            true,
            "public-before-protected",
            "protected-before-private",
            "static-before-instance",
            "variables-before-functions"
        ],
        "new-parens": true,
        "no-any": true,
        "no-arg": true,
        "no-bitwise": true,
        "no-boolean-literal-compare": true,
        "no-conditional-assignment": true,
        "no-console": [true, "log", "error", "debug", "info", "time", "timeEnd", "trace"],
        "no-consecutive-blank-lines": true,
        "no-construct": true,
        "no-debugger": true,
        "no-duplicate-imports": true,
        "no-duplicate-super": true,
        "no-duplicate-switch-case": true,
        "no-duplicate-variable": true,
        "no-empty": true,
        "no-eval": true,
        "no-floating-promises": true,
        "no-for-in-array": true,
        "no-implicit-dependencies": [true, "dev"],
        "no-invalid-template-strings": true,
        "no-invalid-this": true,
        "no-irregular-whitespace": true,
        "no-mergeable-namespace": true,
        "no-misused-new": true,
        "no-null-keyword": true,
        "no-object-literal-type-assertion": true,
        "no-parameter-reassignment": true,
        "no-reference": true,
        "no-return-await": true,
        "no-shadowed-variable": true,
        "no-sparse-arrays": true,
        "no-string-literal": true,
        "no-switch-case-fall-through": true,
        "no-string-throw": true,
        "no-trailing-whitespace": true,
        "no-unnecessary-callback-wrapper": true,
        "no-unnecessary-class": true,
        "no-unnecessary-initializer": true,
        "no-unnecessary-qualifier": true,
        "no-unsafe-finally": true,
        "no-unused-expression": true,
        "no-var-keyword": true,
        "number-literal-format": true,
        "object-literal-key-quotes": [true, "as-needed"],
        "one-line": [true, "check-catch", "check-finally", "check-else", "check-open-brace", "check-whitespace"],
        "prefer-const": true,
        "prefer-for-of": true,
        "prefer-method-signature": true,
        "prefer-object-spread": true,
        "prefer-switch": [true, {"min-cases": 3}],
        "prefer-template": [true, "allow-single-concat"],
        "promise-function-async": true,
        "quotemark": [true, "single", "jsx-double"],
        "radix": true,
        "restrict-plus-operands": true,
        "return-undefined": true,
        "semicolon": [true, "always", "strict-bound-class-methods"],
        "space-before-function-paren": [true, "never"],
        "space-within-parens": false,
        "strict-type-predicates": true,
        "switch-default": true,
        "trailing-comma": [true, {"multiline": "always", "singleline": "never"}],
        "triple-equals": true,
        "typedef": [true, "parameter", "property-declaration"],
        "typedef-whitespace": [
            true,
            {
                "call-signature": "nospace",
                "index-signature": "nospace",
                "parameter": "nospace",
                "property-declaration": "nospace",
                "variable-declaration": "nospace"
            }
        ],
        "use-isnan": true,
        "variable-name": [true, "ban-keywords", "check-format", "allow-leading-underscore", "allow-pascal-case"],
        "whitespace": [
            true,
            "check-branch",
            "check-decl",
            "check-module",
            "check-operator",
            "check-separator",
            "check-rest-spread",
            "check-type",
            "check-typecast",
            "check-type-operator",
            "check-preblock"
        ]
    }
}

Actual behavior

I got an error with the message use undefined instead of null in return null statement of getDerivedStateFromProps.

Expected behavior

According to react documentation getDerivedStateFromProps must return null when there is no actual state change.

I completely understand we must use undefined instead of null in any elsewhere and I know I can ignore the error in that line, but I want to ask to add a feature that tslint-react ignore automatically such special case.

adidahiya commented 5 years ago

Which rule is reporting that error? You can find out using the verbose or stylish formatter.

SaeedZhiany commented 5 years ago

I'm currently traveling right now and can't test. I provide all resources to reproduce the error, can you test it yourself, please?

Only a guess, it's probably produced by "no-null-keyword": true

adidahiya commented 5 years ago

Ok, yeah, it's the no-null-keyword rule. I think this is part of a broader discussion of null restrictions here https://github.com/palantir/tslint/issues/2798

This rule is fairly extreme and probably not suited for general use until we can sort out a good way to deal with the problems described in the linked issue. Closing as a duplicate for now