microsoft / dtslint

A utility built on TSLint for linting TypeScript declaration (.d.ts) files.
MIT License
925 stars 86 forks source link

discuss 'space-before-function-paren' setup according to DT devtool configuration #304

Open peterblazejewicz opened 4 years ago

peterblazejewicz commented 4 years ago

I'd like to discuss addition of rule for dt.json that could one apsect of current setup on the DT with prettier 2. and dtslint. The prettier 2. comes with default, non-confiugrable behavior of space-before-paren: Prettier 2.0 “2020”: Always add a space after the function keyword prettier is now used as the default formatter on the DT project: https://github.com/definitelyTyped/DefinitelyTyped/#common-mistakes https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/.prettierrc.json

The custom rule for space-before-function-paren is not defined in dtslint, so I believe it takes default values: https://palantir.github.io/tslint/rules/space-before-function-paren/ IMO these are:

"space-before-function-paren": [true, {"anonymous": "always", "named": "never", "asyncArrow": "always"}]

Thus the new prettier 2.0 behavior, when applied on anonymous function, creates linter errors, whenever anonymous function is being declared. Here is sample code based on ESLint docs and mentioned prettier release:

// tslint:disable:no-unnecessary-class
// tslint:disable:only-arrow-functions
// tslint:disable:no-unnecessary-generics
// tslint:disable:max-line-length

// eslint sample
// https://eslint.org/docs/rules/space-before-function-paren#anonymous-always-named-never-asyncarrow-always

function foo() {
    // ...
}
const bar = function () {
    //
};
const foobar = function () {
    //
};

class Foo {
    constructor() {
        // ...
    }
}

const fooBar = {
    bar() {
        // ...
    },
};

const fooAsync = async (a: any) => a;

// prettier sample
// https://prettier.io/blog/2020/03/21/2.0.0.html#always-add-a-space-after-the-function-keyword-3903httpsgithubcomprettierprettierpull3903-by-j-f1httpsgithubcomj-f1-josephfrazierhttpsgithubcomjosephfrazier-sosukesuzukihttpsgithubcomsosukesuzuki-thorn0httpsgithubcomthorn0-7516httpsgithubcomprettierprettierpull7516-by-bakkothttpsgithubcombakkot
const identityFunc = function (value: any) {
    return value;
};
function identity(value: any) {
    return value;
}
const f = function <T>(value: T) {};
const g = function* () {};

this results Spaces before function parens are disallowed (space-before-function-paren) 4 errors for:

const bar = function () {
const foobar = function () {
const identityFunc = function (value: any) {
const g = function* () {};

If the anonymous property can be configured as ignore, preserving all others with default values, the linter behaviour can be aligned with both prettier default one and most usecases on DT (functions with both space and no space for anonymous functions):

"space-before-function-paren": [true, { "anonymous": "ignore", "named": "never", "asyncArrow": "always" }]
const bar = function () {
    //
};
const foobar = function() {
    //
};

this will yeld no lint error. So both versions would be allowed to coexists.

Otherwise users will start adding exlusion rules just to correct that prettier 2.0 behavior and linter defualt rules, as experienced here: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/46284#discussion_r460382992 The exclusion rule was added just for fixing linter rule because of the default prettier new behaviour.

Thanks!