From 0fc8f6a9be0e9145158556cdb56d885a9dbbd99f Mon Sep 17 00:00:00 2001 From: Pavel Birukov Date: Wed, 28 Aug 2019 15:47:40 +0300 Subject: [PATCH 1/2] Support granular config in 'object-literal-shorthand' --- src/rules/objectLiteralShorthandRule.ts | 164 ++++++++++++------ .../always/test.ts.lint | 18 +- .../never/test.ts.lint | 26 +-- .../onlyMethods/test.ts.fix | 45 +++++ .../onlyMethods/test.ts.lint | 58 +++++++ .../onlyMethods/tslint.json | 10 ++ 6 files changed, 250 insertions(+), 71 deletions(-) create mode 100644 test/rules/object-literal-shorthand/onlyMethods/test.ts.fix create mode 100644 test/rules/object-literal-shorthand/onlyMethods/test.ts.lint create mode 100644 test/rules/object-literal-shorthand/onlyMethods/tslint.json diff --git a/src/rules/objectLiteralShorthandRule.ts b/src/rules/objectLiteralShorthandRule.ts index 7e3db815242..54bd4532e69 100644 --- a/src/rules/objectLiteralShorthandRule.ts +++ b/src/rules/objectLiteralShorthandRule.ts @@ -28,7 +28,19 @@ import * as ts from "typescript"; import * as Lint from ".."; -const OPTION_NEVER = "never"; +const OPTION_VALUE_NEVER = "never"; +const OPTION_KEY_PROPERTY = "property"; +const OPTION_KEY_METHOD = "method"; + +interface RawOptions { + [OPTION_KEY_PROPERTY]?: "never" | "always"; + [OPTION_KEY_METHOD]?: "never" | "always"; +} + +interface Options { + enforceShorthandMethods: boolean; + enforceShorthandProperties: boolean; +} export class Rule extends Lint.Rules.AbstractRule { /* tslint:disable:object-literal-sort-keys */ @@ -37,12 +49,37 @@ export class Rule extends Lint.Rules.AbstractRule { description: "Enforces/disallows use of ES6 object literal shorthand.", hasFix: true, optionsDescription: Lint.Utils.dedent` - If the \'never\' option is provided, any shorthand object literal syntax will cause a failure.`, + If the \'never\' option is provided, any shorthand object literal syntax will cause a failure. + With \`{"property": "never"}\` provided, the rule fails on property shothands only, + and respectively with \`{"method": "never"}\`, the rule fails only on method shorthands`, options: { - type: "string", - enum: [OPTION_NEVER], + oneOf: [ + { + type: "string", + enum: [OPTION_VALUE_NEVER], + }, + { + type: "object", + properties: { + [OPTION_KEY_PROPERTY]: { + type: "string", + enum: [OPTION_VALUE_NEVER], + }, + [OPTION_KEY_METHOD]: { + type: "string", + enum: [OPTION_VALUE_NEVER], + }, + }, + minProperties: 1, + maxProperties: 2, + }, + ], }, - optionExamples: [true, [true, OPTION_NEVER]], + optionExamples: [ + true, + [true, OPTION_VALUE_NEVER], + [true, { [OPTION_KEY_PROPERTY]: OPTION_VALUE_NEVER }], + ], type: "style", typescriptOnly: false, }; @@ -52,31 +89,94 @@ export class Rule extends Lint.Rules.AbstractRule { public static LONGHAND_METHOD = "Expected method shorthand in object literal "; public static SHORTHAND_ASSIGNMENT = "Shorthand property assignments have been disallowed."; + public static getLonghandPropertyErrorMessage(nodeText: string) { + return `Expected property shorthand in object literal ('${nodeText}').`; + } + public static getLonghandMethodErrorMessage(nodeText: string) { + return `Expected method shorthand in object literal ('${nodeText}').`; + } + public static getDisallowedShorthandErrorMessage(options: Options) { + if (options.enforceShorthandMethods && !options.enforceShorthandProperties) { + return "Shorthand property assignments have been disallowed."; + } else if (!options.enforceShorthandMethods && options.enforceShorthandProperties) { + return "Shorthand method assignments have been disallowed."; + } + return "Shorthand property and method assignments have been disallowed."; + } + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithFunction( - sourceFile, - this.ruleArguments.indexOf(OPTION_NEVER) === -1 - ? enforceShorthandWalker - : disallowShorthandWalker, + return this.applyWithFunction(sourceFile, walk, this.parseOptions(this.ruleArguments)); + } + + private parseOptions(options: Array): Options { + if (options.indexOf(OPTION_VALUE_NEVER) !== -1) { + return { + enforceShorthandMethods: false, + enforceShorthandProperties: false, + }; + } + const optionsObject: RawOptions | undefined = options.find( + (el: string | RawOptions): el is RawOptions => + typeof el === "object" && + (el[OPTION_KEY_PROPERTY] === "never" || el[OPTION_KEY_METHOD] === "never"), ); + if (optionsObject !== undefined) { + return { + enforceShorthandMethods: !(optionsObject[OPTION_KEY_METHOD] === "never"), + enforceShorthandProperties: !(optionsObject[OPTION_KEY_PROPERTY] === "never"), + }; + } else { + return { + enforceShorthandMethods: true, + enforceShorthandProperties: true, + }; + } } } -function disallowShorthandWalker(ctx: Lint.WalkContext) { +function walk(ctx: Lint.WalkContext) { + const { enforceShorthandMethods, enforceShorthandProperties } = ctx.options; return ts.forEachChild(ctx.sourceFile, function cb(node): void { - if (isShorthandPropertyAssignment(node)) { + if ( + enforceShorthandProperties && + isPropertyAssignment(node) && + node.name.kind === ts.SyntaxKind.Identifier && + isIdentifier(node.initializer) && + node.name.text === node.initializer.text + ) { + ctx.addFailureAtNode( + node, + Rule.getLonghandPropertyErrorMessage(`{${node.name.text}}`), + Lint.Replacement.deleteFromTo(node.name.end, node.end), + ); + } else if ( + enforceShorthandMethods && + isPropertyAssignment(node) && + isFunctionExpression(node.initializer) && + // allow named function expressions + node.initializer.name === undefined + ) { + const [name, fix] = handleLonghandMethod(node.name, node.initializer, ctx.sourceFile); + ctx.addFailure( + node.getStart(ctx.sourceFile), + getChildOfKind(node.initializer, ts.SyntaxKind.OpenParenToken, ctx.sourceFile)!.pos, + Rule.getLonghandMethodErrorMessage(`{${name}() {...}}`), + fix, + ); + } else if (!enforceShorthandProperties && isShorthandPropertyAssignment(node)) { ctx.addFailureAtNode( node.name, - Rule.SHORTHAND_ASSIGNMENT, + Rule.getDisallowedShorthandErrorMessage(ctx.options), Lint.Replacement.appendText(node.getStart(ctx.sourceFile), `${node.name.text}: `), ); } else if ( + !enforceShorthandMethods && isMethodDeclaration(node) && node.parent.kind === ts.SyntaxKind.ObjectLiteralExpression ) { ctx.addFailureAtNode( node.name, - Rule.SHORTHAND_ASSIGNMENT, + Rule.getDisallowedShorthandErrorMessage(ctx.options), fixShorthandMethodDeclaration(node, ctx.sourceFile), ); } @@ -84,42 +184,6 @@ function disallowShorthandWalker(ctx: Lint.WalkContext) { }); } -function enforceShorthandWalker(ctx: Lint.WalkContext) { - return ts.forEachChild(ctx.sourceFile, function cb(node): void { - if (isPropertyAssignment(node)) { - if ( - node.name.kind === ts.SyntaxKind.Identifier && - isIdentifier(node.initializer) && - node.name.text === node.initializer.text - ) { - ctx.addFailureAtNode( - node, - `${Rule.LONGHAND_PROPERTY}('{${node.name.text}}').`, - Lint.Replacement.deleteFromTo(node.name.end, node.end), - ); - } else if ( - isFunctionExpression(node.initializer) && - // allow named function expressions - node.initializer.name === undefined - ) { - const [name, fix] = handleLonghandMethod( - node.name, - node.initializer, - ctx.sourceFile, - ); - ctx.addFailure( - node.getStart(ctx.sourceFile), - getChildOfKind(node.initializer, ts.SyntaxKind.OpenParenToken, ctx.sourceFile)! - .pos, - `${Rule.LONGHAND_METHOD}('{${name}() {...}}').`, - fix, - ); - } - } - return ts.forEachChild(node, cb); - }); -} - function fixShorthandMethodDeclaration(node: ts.MethodDeclaration, sourceFile: ts.SourceFile) { const isGenerator = node.asteriskToken !== undefined; const isAsync = hasModifier(node.modifiers, ts.SyntaxKind.AsyncKeyword); diff --git a/test/rules/object-literal-shorthand/always/test.ts.lint b/test/rules/object-literal-shorthand/always/test.ts.lint index bccc19fa851..8e887237eca 100644 --- a/test/rules/object-literal-shorthand/always/test.ts.lint +++ b/test/rules/object-literal-shorthand/always/test.ts.lint @@ -1,10 +1,10 @@ const bad = { w: function() {}, - ~~~~~~~~~~~ [Expected method shorthand in object literal ('{w() {...}}').] + ~~~~~~~~~~~ [LONGHAND_METHOD % ("('{w() {...}}')")] x: function *() {}, - ~~~~~~~~~~~~~ [Expected method shorthand in object literal ('{*x() {...}}').] + ~~~~~~~~~~~~~ [LONGHAND_METHOD % ("('{*x() {...}}')")] [y]: function() {}, - ~~~~~~~~~~~~~ [Expected method shorthand in object literal ('{[y]() {...}}').] + ~~~~~~~~~~~~~ [LONGHAND_METHOD % ("('{[y]() {...}}')")] z: z ~~~~ [Expected property shorthand in object literal ('{z}').] }; @@ -26,7 +26,7 @@ const namedFunctions = { const quotes = { "foo-bar": function() {}, - ~~~~~~~~~~~~~~~~~~~ [Expected method shorthand in object literal ('{"foo-bar"() {...}}').] + ~~~~~~~~~~~~~~~~~~~ [LONGHAND_METHOD % ("('{\"foo-bar\"() {...}}')")] "foo-bar"() {} }; @@ -43,11 +43,13 @@ const extraCases = { const asyncFn = { foo: async function() {}, - ~~~~~~~~~~~~~~~~~~~ [Expected method shorthand in object literal ('{async foo() {...}}').] + ~~~~~~~~~~~~~~~~~~~ [LONGHAND_METHOD % ("('{async foo() {...}}')")] bar: async function*() {} - ~~~~~~~~~~~~~~~~~~~~ [Expected method shorthand in object literal ('{async *bar() {...}}').] + ~~~~~~~~~~~~~~~~~~~~ [LONGHAND_METHOD % ("('{async *bar() {...}}')")] } ({foo: foo} = {foo: foo}); - ~~~~~~~~ [Expected property shorthand in object literal ('{foo}').] - ~~~~~~~~ [Expected property shorthand in object literal ('{foo}').] + ~~~~~~~~ [LONGHAND_PROPERTY % ("('{foo}')")] + ~~~~~~~~ [LONGHAND_PROPERTY % ("('{foo}')")] +[LONGHAND_METHOD]: Expected method shorthand in object literal %s. +[LONGHAND_PROPERTY]: Expected property shorthand in object literal %s. diff --git a/test/rules/object-literal-shorthand/never/test.ts.lint b/test/rules/object-literal-shorthand/never/test.ts.lint index d2d9b546f34..b6ea615c327 100644 --- a/test/rules/object-literal-shorthand/never/test.ts.lint +++ b/test/rules/object-literal-shorthand/never/test.ts.lint @@ -1,31 +1,31 @@ const asyncFn = { async f() { - ~ [OBJECT_LITERAL_DISALLOWED] + ~ [SHORTHAND_ASSIGNMENT] await some_promise; }, async* fa() { - ~~ [OBJECT_LITERAL_DISALLOWED] + ~~ [SHORTHAND_ASSIGNMENT] await some_promise; } }; const bad = { w() { - ~ [OBJECT_LITERAL_DISALLOWED] + ~ [SHORTHAND_ASSIGNMENT] const alsoBad = { bad, - ~~~ [OBJECT_LITERAL_DISALLOWED] + ~~~ [SHORTHAND_ASSIGNMENT] }; }, *x() {}, - ~ [OBJECT_LITERAL_DISALLOWED] + ~ [SHORTHAND_ASSIGNMENT] [y]() {}, - ~~~ [OBJECT_LITERAL_DISALLOWED] + ~~~ [SHORTHAND_ASSIGNMENT] z, - ~ [OBJECT_LITERAL_DISALLOWED] + ~ [SHORTHAND_ASSIGNMENT] nest: { nestBad() {}, - ~~~~~~~ [OBJECT_LITERAL_DISALLOWED] + ~~~~~~~ [SHORTHAND_ASSIGNMENT] nextGood: function(prop: string): void {} } }; @@ -47,12 +47,12 @@ const namedFunctions = { const quotes = { "foo-bar": function() {}, "foo-bar"() {} - ~~~~~~~~~ [OBJECT_LITERAL_DISALLOWED] + ~~~~~~~~~ [SHORTHAND_ASSIGNMENT] }; const extraCases = { x, - ~ [OBJECT_LITERAL_DISALLOWED] + ~ [SHORTHAND_ASSIGNMENT] a: 123, b: "hello", c: 'c', @@ -66,7 +66,7 @@ export class ClassA extends ClassZ { } ({foo} = {foo}); - ~~~ [OBJECT_LITERAL_DISALLOWED] - ~~~ [OBJECT_LITERAL_DISALLOWED] + ~~~ [SHORTHAND_ASSIGNMENT] + ~~~ [SHORTHAND_ASSIGNMENT] -[OBJECT_LITERAL_DISALLOWED]: Shorthand property assignments have been disallowed. +[SHORTHAND_ASSIGNMENT]: Shorthand property and method assignments have been disallowed. diff --git a/test/rules/object-literal-shorthand/onlyMethods/test.ts.fix b/test/rules/object-literal-shorthand/onlyMethods/test.ts.fix new file mode 100644 index 00000000000..47e0f714cf4 --- /dev/null +++ b/test/rules/object-literal-shorthand/onlyMethods/test.ts.fix @@ -0,0 +1,45 @@ +const badMethodsGoodProps = { + w() {}, + *x() {}, + [y]() {}, + z: z +}; + +const goodMethodsBadProps = { + w() {}, + *x() {}, + [y]() {}, + z: z +}; + +const arrows = { + x: (y) => y // this is OK. +}; + +const namedFunctions = { + x: function y() {} // named function expressions are also OK. +}; + +const quotes = { + "foo-bar"() {}, + "foo-bar"() {} +}; + +const extraCases = { + x: x, + a: 123, + b: "hello", + c: 'c', + ["a" + "nested"]: { + x: x + } +}; + +const asyncFn = { + async foo() {}, + async *bar() {} +} + +({foo: foo} = {foo: foo}); +({foo: foo} = {foo: foo}); + diff --git a/test/rules/object-literal-shorthand/onlyMethods/test.ts.lint b/test/rules/object-literal-shorthand/onlyMethods/test.ts.lint new file mode 100644 index 00000000000..ffc9981c5d1 --- /dev/null +++ b/test/rules/object-literal-shorthand/onlyMethods/test.ts.lint @@ -0,0 +1,58 @@ +const badMethodsGoodProps = { + w: function() {}, + ~~~~~~~~~~~ [LONGHAND_METHOD % ("('{w() {...}}')")] + x: function *() {}, + ~~~~~~~~~~~~~ [LONGHAND_METHOD % ("('{*x() {...}}')")] + [y]: function() {}, + ~~~~~~~~~~~~~ [LONGHAND_METHOD % ("('{[y]() {...}}')")] + z: z +}; + +const goodMethodsBadProps = { + w() {}, + *x() {}, + [y]() {}, + z + ~ [OBJECT_LITERAL_DISALLOWED] +}; + +const arrows = { + x: (y) => y // this is OK. +}; + +const namedFunctions = { + x: function y() {} // named function expressions are also OK. +}; + +const quotes = { + "foo-bar": function() {}, + ~~~~~~~~~~~~~~~~~~~ [LONGHAND_METHOD % ("('{\"foo-bar\"() {...}}')")] + "foo-bar"() {} +}; + +const extraCases = { + x, + ~ [OBJECT_LITERAL_DISALLOWED] + a: 123, + b: "hello", + c: 'c', + ["a" + "nested"]: { + x: x + } +}; + +const asyncFn = { + foo: async function() {}, + ~~~~~~~~~~~~~~~~~~~ [LONGHAND_METHOD % ("('{async foo() {...}}')")] + bar: async function*() {} + ~~~~~~~~~~~~~~~~~~~~ [LONGHAND_METHOD % ("('{async *bar() {...}}')")] +} + +({foo: foo} = {foo: foo}); +({foo} = {foo}); + ~~~ [OBJECT_LITERAL_DISALLOWED] + ~~~ [OBJECT_LITERAL_DISALLOWED] + +[OBJECT_LITERAL_DISALLOWED]: Shorthand property assignments have been disallowed. +[LONGHAND_METHOD]: Expected method shorthand in object literal %s. +[LONGHAND_PROPERTY]: Expected property shorthand in object literal %s. diff --git a/test/rules/object-literal-shorthand/onlyMethods/tslint.json b/test/rules/object-literal-shorthand/onlyMethods/tslint.json new file mode 100644 index 00000000000..bb3baad7359 --- /dev/null +++ b/test/rules/object-literal-shorthand/onlyMethods/tslint.json @@ -0,0 +1,10 @@ +{ + "rules": { + "object-literal-shorthand": [ + true, + { + "property": "never" + } + ] + } +} From e4b41235657626df0f31fa988434645efcd8aeb6 Mon Sep 17 00:00:00 2001 From: Pavel Birukov Date: Thu, 5 Sep 2019 14:19:55 +0300 Subject: [PATCH 2/2] Improve wording and remove string literals from 'object-literal-shorthand' --- src/rules/objectLiteralShorthandRule.ts | 21 +++++++++++++------ .../onlyMethods/tslint.json | 3 ++- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/rules/objectLiteralShorthandRule.ts b/src/rules/objectLiteralShorthandRule.ts index 54bd4532e69..c3d875ac7b2 100644 --- a/src/rules/objectLiteralShorthandRule.ts +++ b/src/rules/objectLiteralShorthandRule.ts @@ -49,9 +49,15 @@ export class Rule extends Lint.Rules.AbstractRule { description: "Enforces/disallows use of ES6 object literal shorthand.", hasFix: true, optionsDescription: Lint.Utils.dedent` - If the \'never\' option is provided, any shorthand object literal syntax will cause a failure. - With \`{"property": "never"}\` provided, the rule fails on property shothands only, - and respectively with \`{"method": "never"}\`, the rule fails only on method shorthands`, + \`"always"\` assumed to be default option, thus with no options provided + the rule enforces object literal methods and properties shorthands. + With \`"never"\` option provided, any shorthand object literal syntax causes an error. + + The rule can be configured in a more granular way. + With \`{"property": "never"}\` provided (which is equivalent to \`{"property": "never", "method": "always"}\`), + the rule only flags property shorthand assignments, + and respectively with \`{"method": "never"}\` (equivalent to \`{"property": "always", "method": "never"}\`), + the rule fails only on method shorthands.`, options: { oneOf: [ { @@ -118,12 +124,15 @@ export class Rule extends Lint.Rules.AbstractRule { const optionsObject: RawOptions | undefined = options.find( (el: string | RawOptions): el is RawOptions => typeof el === "object" && - (el[OPTION_KEY_PROPERTY] === "never" || el[OPTION_KEY_METHOD] === "never"), + (el[OPTION_KEY_PROPERTY] === OPTION_VALUE_NEVER || + el[OPTION_KEY_METHOD] === OPTION_VALUE_NEVER), ); if (optionsObject !== undefined) { return { - enforceShorthandMethods: !(optionsObject[OPTION_KEY_METHOD] === "never"), - enforceShorthandProperties: !(optionsObject[OPTION_KEY_PROPERTY] === "never"), + enforceShorthandMethods: !(optionsObject[OPTION_KEY_METHOD] === OPTION_VALUE_NEVER), + enforceShorthandProperties: !( + optionsObject[OPTION_KEY_PROPERTY] === OPTION_VALUE_NEVER + ), }; } else { return { diff --git a/test/rules/object-literal-shorthand/onlyMethods/tslint.json b/test/rules/object-literal-shorthand/onlyMethods/tslint.json index bb3baad7359..fc32bc1d466 100644 --- a/test/rules/object-literal-shorthand/onlyMethods/tslint.json +++ b/test/rules/object-literal-shorthand/onlyMethods/tslint.json @@ -3,7 +3,8 @@ "object-literal-shorthand": [ true, { - "property": "never" + "property": "never", + "method": "always" } ] }