Skip to content
This repository was archived by the owner on Mar 25, 2021. It is now read-only.

Add 'object-literal-contextual-type' rule#2581

Closed
andy-hanson wants to merge 4 commits intopalantir:masterfrom
andy-hanson:object-literal-contextual-type
Closed

Add 'object-literal-contextual-type' rule#2581
andy-hanson wants to merge 4 commits intopalantir:masterfrom
andy-hanson:object-literal-contextual-type

Conversation

@andy-hanson
Copy link
Contributor

PR checklist

  • Addresses an existing issue: #0000
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Overview of change:

Added the object-literal-contextual-type rule, which warns for an object literal with no contextual type (which may have unchecked excess properties).
This branch is based on top of #2544 and will be rebased once that's in.

CHANGELOG.md entry:

[new-rule] object-literal-contextual-type

@andy-hanson andy-hanson changed the title Object literal contextual type Add 'object-literal-contextual-type' rule Apr 16, 2017
@andy-hanson andy-hanson force-pushed the object-literal-contextual-type branch 3 times, most recently from 6ad5b97 to b1707fb Compare April 18, 2017 01:17
@andy-hanson
Copy link
Contributor Author

Rebased.

@andy-hanson andy-hanson force-pushed the object-literal-contextual-type branch from b1707fb to cdd9eef Compare April 18, 2017 01:20
Copy link

@giladgray giladgray left a comment

Choose a reason for hiding this comment

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

typescript has gotten better at handling objects like this. this rule is too aggressive (requiring too many changes and disables in existing source code).

};

public static ARGUMENT_DESCRIPTOR_BLOCK = {
public static ARGUMENT_DESCRIPTOR_BLOCK: {} = {

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

{} is a correct type. Anything but null or undefined or void is assignable to {}.

* limitations under the License.
*/

// tslint:disable object-literal-contextual-type

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.

Copy link
Contributor Author

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?

interface I { x: number; }
function f(): I {
const res = { x: 0, y: 0 };
return res;

Choose a reason for hiding this comment

The 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...

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.
Often people will think they're getting a contextual type when they really aren't, e.g. a result from a callback:

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 (n): Options, but I wouldn't want a lint rule to require all arrow functions to have explicit return types.
If there are better ways of avoiding this error I'd like to hear about them.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants