Flow ruleset #22
Flow ruleset #22
Conversation
|
Thanks for this! I have some suggestions/requests to amend the current structure of the new ruleset. There are quite several coding-style specific rules which deal with how the code looks like, instead of how it works. I would like to move all such rules to a new coding-styles/flow ruleset (just like we have for react). I will go through the existing rules and see if I am okay with their current configuration. In the meantime, feel free to start moving the rules out. Thank you! |
environments/flow/optional.js
Outdated
| 'flowtype/boolean-style': ['warn', 'boolean'], | ||
|
|
||
| // Disallows use of primitive constructors as types, such as Boolean, Number and String. | ||
| 'flowtype/no-primitive-constructor-types': 'warn', |
There was a problem hiding this comment.
Change to error. The primitive constructors are almost never used, only when type-casting, ie.
const bool = Boolean('I am Groot') // true
const num = Number('42') // 42 instead of "42"
// etc.There was a problem hiding this comment.
Also, move from optional to recommended.
environments/flow/optional.js
Outdated
|
|
||
| // Enforces sorting of Object annotations. | ||
| // This rule mirrors ESlint's sort-keys rule. | ||
| 'flowtype/sort-keys': 'warn', |
There was a problem hiding this comment.
Probably too restrictive even for a coding-style ruleset. Please move to unused.js and set to 0.
There was a problem hiding this comment.
Because after all changes environments/flow/optional.js contains only this rule and flowtype/type-id-match I would left them as optional part of ruleset.
environments/flow/optional.js
Outdated
| 'flowtype/sort-keys': 'warn', | ||
|
|
||
| // Enforces a consistent naming pattern for type aliases. | ||
| 'flowtype/type-id-match': ['warn', '^([A-Z][a-z0-9]*)+Type$'], |
There was a problem hiding this comment.
Looks too restrictive, having Type suffix in all type defs feels ugly. For now, let's move this to unused.js, we can re-visit later.
There was a problem hiding this comment.
Because after all changes environments/flow/optional.js contains only this rule and flowtype/sort-keys I would left them as optional part of ruleset.
environments/flow/recommended.js
Outdated
| /** | ||
| * Js-coding-standards | ||
| * | ||
| * @author Robert Rossmann <rr.rossmann@me.com> |
There was a problem hiding this comment.
Uhm, change to Arnost Pleskot? 😄
environments/flow/recommended.js
Outdated
| * Js-coding-standards | ||
| * | ||
| * @author Robert Rossmann <rr.rossmann@me.com> | ||
| * @copyright 2016 STRV |
environments/flow/recommended.js
Outdated
| 'flowtype/space-after-type-colon': ['error', 'always'], | ||
|
|
||
| // Enforces consistent spacing before the opening < of generic type annotation parameters. | ||
| 'flowtype/space-before-generic-bracket': ['error', 'always'], |
There was a problem hiding this comment.
coding-style -> warn
Also I think the space before generic annotation is rarely used in other languages, so please switch to never.
environments/flow/recommended.js
Outdated
| 'flowtype/space-before-generic-bracket': ['error', 'always'], | ||
|
|
||
| // Enforces consistent spacing before the type annotation colon. | ||
| 'flowtype/space-before-type-colon': ['error', 'never'], |
environments/flow/recommended.js
Outdated
| 'flowtype/space-before-type-colon': ['error', 'never'], | ||
|
|
||
| // Enforces consistent spacing around union and intersection type separators (` | ` and ` & `). | ||
| 'flowtype/union-intersection-spacing': ['error', 'always'], |
environments/flow/optional.js
Outdated
|
|
||
| rules: { | ||
| // Enforces a particular style for boolean type annotations. | ||
| 'flowtype/boolean-style': ['warn', 'boolean'], |
There was a problem hiding this comment.
I think this could be a recommended rule with these settings. Consistency is of great importance.
environments/flow/optional.js
Outdated
|
|
||
| // Warns against weak type annotations any, Object and Function. These types can cause flow | ||
| // to silently skip over portions of your code, which would have otherwise caused type errors. | ||
| 'flowtype/no-weak-types': ['warn', { any: true }], |
There was a problem hiding this comment.
Move to recommended with these settings.
|
Merged, awesome - thanks! 🎉 🍻 |
Added eslint-plugin-flowtype and related rules.