-
Notifications
You must be signed in to change notification settings - Fork 878
Add 'object-literal-contextual-type' rule #2581
Changes from all commits
cdd9eef
9e37ac1
814afeb
fd5b679
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -109,7 +109,7 @@ export class Rule extends Lint.Rules.TypedRule { | |
| }, | ||
| }; | ||
|
|
||
| public static ARGUMENT_DESCRIPTOR_BLOCK = { | ||
| public static ARGUMENT_DESCRIPTOR_BLOCK: {} = { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this isn't even correct. it has a much more advanced type than the empty object.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| properties: { | ||
| [DESCRIPTOR_TAGS]: { | ||
| properties: { | ||
|
|
@@ -139,7 +139,7 @@ export class Rule extends Lint.Rules.TypedRule { | |
| type: "object", | ||
| }; | ||
|
|
||
| public static ARGUMENT_DESCRIPTOR_CLASS = { | ||
| public static ARGUMENT_DESCRIPTOR_CLASS: {} = { | ||
| properties: { | ||
| [DESCRIPTOR_TAGS]: { | ||
| properties: { | ||
|
|
@@ -285,7 +285,7 @@ export class Rule extends Lint.Rules.TypedRule { | |
| typescriptOnly: false, | ||
| requiresTypeInfo: true, | ||
| }; | ||
| /* tslint:enable:object-literal-sort-keys */ | ||
| /* tslint:enable:object-literal-sort-keys object-literal-contextual-type */ | ||
|
|
||
| private readonly exclusionFactory = new ExclusionFactory(); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2017 Palantir Technologies, Inc. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| import { isArrayLiteralExpression, isObjectLiteralExpression, isPropertyAssignment } from "tsutils"; | ||
| import * as ts from "typescript"; | ||
|
|
||
| import * as Lint from "../index"; | ||
|
|
||
| export class Rule extends Lint.Rules.TypedRule { | ||
| /* tslint:disable:object-literal-sort-keys */ | ||
| public static metadata: Lint.IRuleMetadata = { | ||
| ruleName: "object-literal-contextual-type", | ||
| description: "Requires that every object literal has a contextual type.", | ||
| rationale: Lint.Utils.dedent` | ||
| An object literal with no type does not have excess properties checked. | ||
|
|
||
| For example: | ||
|
|
||
| interface I { x: number; } | ||
| function f(): I { | ||
| const res = { x: 0, y: 0 }; | ||
| return res; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this error can be avoided so many ways that don't require a rather aggressive lint rule...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would it be avoided in general? I don't believe typescript's behavior with respect to object literals has changed much in the past year. interface Options { foo?: number; bar?: number }
const arr: Options[] = [1, 2, 3].map(n => ({ foo: n, baa: n }));The error is fixed by changing to |
||
| } | ||
|
|
||
| This has no compile error, but an excess property \`y\`. | ||
| The excess property can be detected by writing a type annotation \`const res: I = { x: 0, y: 0 };\`.`, | ||
| optionsDescription: "Not configurable.", | ||
| options: null, | ||
| optionExamples: [true], | ||
| type: "functionality", | ||
| typescriptOnly: true, | ||
| requiresTypeInfo: true, | ||
| }; | ||
| /* tslint:enable:object-literal-sort-keys */ | ||
|
|
||
| public static FAILURE_STRING = "Object literal has no contextual type."; | ||
|
|
||
| public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { | ||
| return this.applyWithFunction(sourceFile, (ctx) => walk(ctx, program.getTypeChecker())); | ||
| } | ||
| } | ||
|
|
||
| function walk(ctx: Lint.WalkContext<void>, checker: ts.TypeChecker): void { | ||
| ts.forEachChild(ctx.sourceFile, function cb(node) { | ||
| if (isObjectLiteralExpression(node)) { | ||
| check(node); | ||
| } | ||
| ts.forEachChild(node, cb); | ||
| }); | ||
|
|
||
| function check(node: ts.ObjectLiteralExpression): void { | ||
| // Allow `{}`, because obviously it does not have excess properties. | ||
| if (node.properties.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| // Allow an object literal inside another object literal or array literal (recursively) typed as 'any'. | ||
| // Normally the nested objects will not have a contextual type, so must traverse upwards to look for it. | ||
| let contextualNode: ts.Expression = node; | ||
|
|
||
| do { | ||
| if (checker.getContextualType(contextualNode) !== undefined) { | ||
| return; | ||
| } | ||
|
|
||
| const parent = contextualNode.parent!; | ||
| if (isPropertyAssignment(parent)) { | ||
| contextualNode = parent.parent; | ||
| } else if (isArrayLiteralExpression(parent)) { | ||
| contextualNode = parent; | ||
| } else { | ||
| ctx.addFailureAtNode(node, Rule.FAILURE_STRING); | ||
| return; | ||
| } | ||
| } while (true); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| const ok: { x: 0 } = { x: 0 }; | ||
|
|
||
| const notOk = { x: 0 }; | ||
| ~~~~~~~~ [0] | ||
|
|
||
| // `{}` OK. | ||
| const empty = {}; | ||
|
|
||
| declare function f(obj: { x: number }): void; | ||
| f({ x: 0 }); | ||
|
|
||
| // Allow any literal of type 'any', | ||
| // or nested inside an array or object literal in something else of type 'any'. | ||
| const isAny: any = { | ||
| a: { x: 0 }, | ||
| b: [ | ||
| { x: 0 }, | ||
| ], | ||
| c() { | ||
| return { x: 0 }; | ||
| ~~~~~~~~ [0] | ||
| }, | ||
| }; | ||
|
|
||
| [0]: Object literal has no contextual type. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| { | ||
| "rules": { | ||
| "object-literal-contextual-type": true | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API break!! not down for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a comment, how does it break the API?