New: Add rule no-type-literal #15

weirdpattern · 2017-01-05T03:22:41Z

This includes

New rule no-type-literal
Corresponding test
Corresponding documentation
Update to README.md to include the new rule

weirdpattern · 2017-01-05T03:24:49Z

Hi @dannyfritz, could you please reference this PR from #5
Also, please let me know if you need a hand with the some of these rules, I'll be glad to help!

dannyfritz · 2017-01-05T04:42:56Z

Which TSLint does this correlate to?

EDIT: found it in your docs 😄 interface-over-type-literal

JamesHenry · 2017-01-14T17:04:13Z

docs/rules/no-type-literal.md

@@ -0,0 +1,121 @@
+# Enforces the use of interfaces over type literals (`type T = {...}`).


TypeScript refers to these as "Type Aliases" in their docs, so please could we go for that name

correct me if I'm wrong, isn't type aliases a much broader concept? a type alias could be:

an actual alias type foo = string | string[];

a callback type foo = () => 'hello';

a literal type foo = {};

I used type literals, because the rule only covers the last case...

anyway I'm ok changing the text, just want to make sure you are ok with this

(Sorry not on computer right now, so can't do much digging)

We have the "allowUnion" and "allowIntersection" configuration in this rule which broadens it from just a literal, right? Plus should we not actually be including your callback example in this rule as well?

e.g. should we not be preferring func2 here?

https://www.typescriptlang.org/play/index.html#src=type%20func%20%3D%20()%20%3D%3E%20true%0D%0A%0D%0Ainterface%20func2%20%7B%0D%0A%20%20%20%20()%3A%20true%3B%0D%0A%7D%0D%0A%0D%0Aconst%20test%3A%20func%20%3D%20()%20%3D%3E%20true%0D%0Aconst%20test2%3A%20func2%20%3D%20()%20%3D%3E%20true

this rule is based on the tslint rule (which includes only the literals), but yeah, we can broaden the scope by including the callback too. I think we should consider renaming the rule, though...

thoughts?

by the way, the allowUnion and allowIntersection enable compositions that include a literal type. e.g.
type foo = string & {};

now I'm not so sure if we still need them if we go with literals and callbacks. I think we can rename the rule to no-type-aliases, with two properties allowTypeLiterals and allowCallbacks

JamesHenry · 2017-01-14T17:04:55Z

docs/rules/no-type-literal.md

+
+This rule aims to standardise the use of interfaces across the codebase.
+
+## Options


As with the other PR, please can we stick to the exceptions being false by default for "no-" rules, I feel it is much clearer to users of the rule.

JamesHenry

You're killing it with these PRs @weirdpattern, thank you!

Just some minor docs tweaks again

weirdpattern · 2017-01-19T21:20:37Z

@JamesHenry I did a couple of modifications to the rule based on the discussion we had few days ago.
Let me know what you think, I believe I cover all possible scenarios, but who knows...

NeoReyad · 2017-01-19T21:54:52Z

@weirdpattern Would it be possible to ignore mapped types (https://www.typescriptlang.org/docs/handbook/advanced-types.html#mapped-types)? Not sure if this has been discussed yet or implemented in the current rule.

weirdpattern · 2017-01-19T22:18:46Z

@NeoReyad nope, I did not include mapped types... but it shouldn't be too complicated to implementing.
Now, I know @JamesHenry is waiting on this rule to be merged to do a release, so maybe we can do this in v2 of the rule?

What do you guys think? Or if you guys want to wait a little longer I can work on this in the next few days...

weirdpattern · 2017-01-19T22:48:01Z

on second thought, I think I'm gonna work on this tonight... I just realized the code doesn't handle those scenarios properly...

weirdpattern · 2017-01-20T04:19:57Z

Ok, I added allowMappedTypes to the supported options.
If there are no more comments, this is ready for review!

weirdpattern · 2017-01-20T04:25:58Z

Build is failing due to typescript version (2.0.3), works fine with latest version (2.1.x) as mapped types were introduced in this version release notes

JamesHenry · 2017-01-20T09:53:24Z

I'm sorry @weirdpattern but we cannot support TS 2.1.x in the parser yet - it is blocked on a TypeScript bug.

See the upgrade issue for more details: eslint/typescript-eslint-parser#128

You will either have to remove the mapped types again or we can wait until after that issue is resolved to move forward with this PR. I'll leave it up to you!

NeoReyad · 2017-01-20T14:26:46Z

@weirdpattern I really appreciate the work you done on this plugin and others. Thanks for taking the time to include mapped types. I have looked into the typescript JsDoc bug that is blocking the parser from upgrading. I will try and get a pull request submitted this weekend. Hopefully we can have TypeScript patched and released in a few weeks.

I think the only part that is breaking is the tests and not the plugin itself since it does not depend on SyntaxKind only the node.type string. Maybe we could just make those tests conditional?

If you decided to remove the feature that is fine too but it'd be cool if we could include it in a separate typescript 2.1 branch, like we currently do with the parser. This way I could use the mapped type rule today instead of forking the project in my own repo.

Thanks again.

weirdpattern · 2017-01-20T14:55:58Z

@NeoReyad it looks like it's going to be a while before we can move to v2.1.x. This bug was labeled TypeScript 2.2 and there is no due date yet...

Re creating a 2.1 branch for this rule (and any other requiring v2.1.x), I'm all for it, but I'm just a contributor here, so I cannot create branches on this repo. I leave the decision to @nzakas and/or @JamesHenry, for now I'm just going to leave this PR as is.

NeoReyad · 2017-01-20T18:36:10Z

I see they have marked it as 2.2 milestone which i think is scheduled for march. One of the developer comments mentioned a quick fix. I will see if they can apply it to the 2.1 branch.

weirdpattern · 2017-02-02T06:26:03Z

I guess I can remove the mapped types from this PR, then create a new one that includes everything. The latter can be merged once TS 2.2 gets released and the parser supports it.

I will work on this tomorrow.

weirdpattern · 2017-02-03T20:49:31Z

@JamesHenry I removed mapped types from this PR, we can merge now.
I will submit a new PR and am going to mark it as 2.1

JamesHenry · 2017-07-11T11:35:27Z

Hey @weirdpattern, I am sorry for the insane delay on this. I have just merged in some important updates to the plugin from @corbinu, and released as 0.3.0.

Are you available to update this PR to include all the latest work?

- rename from no-type-literals to no-type-alias (thus broadening the scope of the rule). - rule now supports 4 configurations (allowAliases, allowCallbacks, allowLiterals and allowMappedTypes). - allowAliases supports values: true, false (and synonyms "always", "never"), "in-unions", "in-intersections", "in-unions-and-intersections". - allowCallbacks supports values: true, false (and synonyms "always", "never"). - allowLiterals supports values: true, false (and synonyms "always", "never"), "in-unions", "in-intersections", "in-unions-and-intersections". - allowMappedTypes supports values: true, false (and synonyms "always", "never"), "in-unions", "in-intersections", "in-unions-and-intersections". - add tests for each case and combinations - update documentation Remove mapped types as it's not currently supported. Add support for mapped types and aliases

weirdpattern · 2017-07-12T02:54:48Z

@JamesHenry and @corbinu I've added the missing mapped types (see Feb 3 comment few lines above) and rebase the code to include the latest... we should be good to go, let me know otherwise...

JamesHenry · 2017-07-18T15:48:35Z

Thanks, @weirdpattern!

weirdpattern changed the title ~~New: Add rule no-type-literal.md~~ New: Add rule no-type-literal Jan 5, 2017

dannyfritz mentioned this pull request Jan 5, 2017

[META] TSLint Compatibility #5

Closed

31 tasks

JamesHenry reviewed Jan 14, 2017

View reviewed changes

JamesHenry suggested changes Jan 14, 2017

View reviewed changes

weirdpattern added 3 commits July 11, 2017 21:46

Resolving conflicts in README.md

2f77064

Updating formatting to comply with eslint rules

2d06065

weirdpattern force-pushed the no-type-literal branch from c205a18 to 2d06065 Compare July 12, 2017 02:51

JamesHenry approved these changes Jul 18, 2017

View reviewed changes

JamesHenry merged commit d7b7936 into bradzacher:master Jul 18, 2017

weirdpattern deleted the no-type-literal branch August 4, 2017 01:57

		@@ -0,0 +1,121 @@
		# Enforces the use of interfaces over type literals (`type T = {...}`).


		This rule aims to standardise the use of interfaces across the codebase.

		## Options

New: Add rule no-type-literal #15

New: Add rule no-type-literal #15

Uh oh!

Conversation

weirdpattern commented Jan 5, 2017

Uh oh!

weirdpattern commented Jan 5, 2017

Uh oh!

dannyfritz commented Jan 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JamesHenry Jan 14, 2017

Choose a reason for hiding this comment

Uh oh!

weirdpattern Jan 14, 2017

Choose a reason for hiding this comment

Uh oh!

JamesHenry Jan 14, 2017

Choose a reason for hiding this comment

Uh oh!

weirdpattern Jan 14, 2017

Choose a reason for hiding this comment

Uh oh!

weirdpattern Jan 14, 2017

Choose a reason for hiding this comment

Uh oh!

JamesHenry Jan 14, 2017

Choose a reason for hiding this comment

Uh oh!

JamesHenry left a comment

Choose a reason for hiding this comment

Uh oh!

weirdpattern commented Jan 19, 2017

Uh oh!

NeoReyad commented Jan 19, 2017

Uh oh!

weirdpattern commented Jan 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weirdpattern commented Jan 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weirdpattern commented Jan 20, 2017

Uh oh!

weirdpattern commented Jan 20, 2017

Uh oh!

JamesHenry commented Jan 20, 2017

Uh oh!

NeoReyad commented Jan 20, 2017

Uh oh!

weirdpattern commented Jan 20, 2017

Uh oh!

NeoReyad commented Jan 20, 2017

Uh oh!

weirdpattern commented Feb 2, 2017

Uh oh!

weirdpattern commented Feb 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JamesHenry commented Jul 11, 2017

Uh oh!

weirdpattern commented Jul 12, 2017

Uh oh!

JamesHenry commented Jul 18, 2017

Uh oh!

Uh oh!

dannyfritz commented Jan 5, 2017 •

edited

Loading

weirdpattern commented Jan 19, 2017 •

edited

Loading

weirdpattern commented Jan 19, 2017 •

edited

Loading

weirdpattern commented Feb 3, 2017 •

edited

Loading