diff --git a/.README/README.md b/.README/README.md index f4e7c8c..e6c3729 100644 --- a/.README/README.md +++ b/.README/README.md @@ -194,4 +194,5 @@ When `true`, only checks files with a [`@flow` annotation](http://flowtype.org/d {"gitdown": "include", "file": "./rules/type-import-style.md"} {"gitdown": "include", "file": "./rules/union-intersection-spacing.md"} {"gitdown": "include", "file": "./rules/use-flow-type.md"} +{"gitdown": "include", "file": "./rules/use-read-only-spread.md"} {"gitdown": "include", "file": "./rules/valid-syntax.md"} diff --git a/.README/rules/use-read-only-spread.md b/.README/rules/use-read-only-spread.md new file mode 100644 index 0000000..7658f39 --- /dev/null +++ b/.README/rules/use-read-only-spread.md @@ -0,0 +1,39 @@ +### `use-read-only-spread` + +Warns against accidentally creating an object which is no longer read-only because of how spread operator works in Flow. Imagine the following code: + +```flow js +type INode = {| + +type: string, +|}; + +type Identifier = {| + ...INode, + +name: string, +|}; +``` + +You might expect the identifier name to be read-only, however, that's not true ([flow.org/try](https://flow.org/try/#0C4TwDgpgBAkgcgewCbQLxQN4B8BQUoDUokAXFAM7ABOAlgHYDmANDlgL4DcOOx0MKdYDQBmNCFSjpseKADp58ZBBb4CdAIYBbCGUq1GLdlxwBjBHUpQAHmX4RBIsRKlQN2sgHIPTKL08eoTm4rWV5JKA8AZQALBABXABskVwRgKAAjaAB3WmB1dISIAEIPLhC3NAiY+KSUtMyoHJo8guLSnCA)): + +```flow js +const x: Identifier = { name: '', type: '' }; + +x.type = 'Should not be writable!'; // No Flow error +x.name = 'Should not be writable!'; // No Flow error +``` + +This rule suggests to use `$ReadOnly<…>` to prevent accidental loss of readonly-ness: + +```flow js +type Identifier = $ReadOnly<{| + ...INode, + +name: string, +|}>; + +const x: Identifier = { name: '', type: '' }; + +x.type = 'Should not be writable!'; // $FlowExpectedError[cannot-write] +x.name = 'Should not be writable!'; // $FlowExpectedError[cannot-write] +``` + + diff --git a/README.md b/README.md index 9b2f188..276ceb1 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,7 @@ * [`type-import-style`](#eslint-plugin-flowtype-rules-type-import-style) * [`union-intersection-spacing`](#eslint-plugin-flowtype-rules-union-intersection-spacing) * [`use-flow-type`](#eslint-plugin-flowtype-rules-use-flow-type) + * [`use-read-only-spread`](#eslint-plugin-flowtype-rules-use-read-only-spread) * [`valid-syntax`](#eslint-plugin-flowtype-rules-valid-syntax) @@ -6430,6 +6431,105 @@ import type A from "a"; type X> = { b: B }; let x: X; console.log( + +### use-read-only-spread + +Warns against accidentally creating an object which is no longer read-only because of how spread operator works in Flow. Imagine the following code: + +```flow js +type INode = {| + +type: string, +|}; + +type Identifier = {| + ...INode, + +name: string, +|}; +``` + +You might expect the identifier name to be read-only, however, that's not true ([flow.org/try](https://flow.org/try/#0C4TwDgpgBAkgcgewCbQLxQN4B8BQUoDUokAXFAM7ABOAlgHYDmANDlgL4DcOOx0MKdYDQBmNCFSjpseKADp58ZBBb4CdAIYBbCGUq1GLdlxwBjBHUpQAHmX4RBIsRKlQN2sgHIPTKL08eoTm4rWV5JKA8AZQALBABXABskVwRgKAAjaAB3WmB1dISIAEIPLhC3NAiY+KSUtMyoHJo8guLSnCA)): + +```flow js +const x: Identifier = { name: '', type: '' }; + +x.type = 'Should not be writable!'; // No Flow error +x.name = 'Should not be writable!'; // No Flow error +``` + +This rule suggests to use `$ReadOnly<…>` to prevent accidental loss of readonly-ness: + +```flow js +type Identifier = $ReadOnly<{| + ...INode, + +name: string, +|}>; + +const x: Identifier = { name: '', type: '' }; + +x.type = 'Should not be writable!'; // $FlowExpectedError[cannot-write] +x.name = 'Should not be writable!'; // $FlowExpectedError[cannot-write] +``` + +The following patterns are considered problems: + +```js +type INode = {||}; +type Identifier = {| + ...INode, + +aaa: string, +|}; +// Message: Flow type with spread property and all readonly properties should be wrapped in '$ReadOnly<…>' to prevent accidental loss of readonly-ness. + +type INode = {||}; +type Identifier = {| + ...INode, + +aaa: string, + +bbb: string, +|}; +// Message: Flow type with spread property and all readonly properties should be wrapped in '$ReadOnly<…>' to prevent accidental loss of readonly-ness. +``` + +The following patterns are not considered problems: + +```js +type INode = {||}; +type Identifier = {| + ...INode, + name: string, +|}; + +type INode = {||}; +type Identifier = {| + ...INode, + name: string, // writable on purpose + +surname: string, +|}; + +type Identifier = {| + +name: string, +|}; + +type INode = {||}; +type Identifier = $ReadOnly<{| + ...INode, + +name: string, +|}>; + +type INode = {||}; +type Identifier = $ReadOnly<{| + ...INode, + name: string, // writable on purpose +|}>; + +type INode = {||}; +type Identifier = $ReadOnly<{| + ...INode, + -name: string, +|}>; +``` + + + ### valid-syntax diff --git a/src/index.js b/src/index.js index f4394e9..06c251e 100644 --- a/src/index.js +++ b/src/index.js @@ -39,6 +39,7 @@ import typeIdMatch from './rules/typeIdMatch'; import typeImportStyle from './rules/typeImportStyle'; import unionIntersectionSpacing from './rules/unionIntersectionSpacing'; import useFlowType from './rules/useFlowType'; +import useReadOnlySpread from './rules/useReadOnlySpread'; import validSyntax from './rules/validSyntax'; import spreadExactType from './rules/spreadExactType'; import arrowParens from './rules/arrowParens'; @@ -85,6 +86,7 @@ const rules = { 'type-import-style': typeImportStyle, 'union-intersection-spacing': unionIntersectionSpacing, 'use-flow-type': useFlowType, + 'use-read-only-spread': useReadOnlySpread, 'valid-syntax': validSyntax, }; diff --git a/src/rules/useReadOnlySpread.js b/src/rules/useReadOnlySpread.js new file mode 100644 index 0000000..8604bbc --- /dev/null +++ b/src/rules/useReadOnlySpread.js @@ -0,0 +1,44 @@ +const meta = { + messages: { + readonlySpread: + 'Flow type with spread property and all readonly properties should be ' + + 'wrapped in \'$ReadOnly<…>\' to prevent accidental loss of readonly-ness.', + }, +}; + +const create = (context) => { + return { + TypeAlias (node) { + if (node.right.type === 'GenericTypeAnnotation' && node.right.id.name === '$ReadOnly') { + // it's already $ReadOnly<…>, nothing to do + } else if (node.right.type === 'ObjectTypeAnnotation') { + // let's iterate all props and if everything is readonly then throw + let shouldThrow = false; + let hasSpread = false; + for (const property of node.right.properties) { + if (property.type === 'ObjectTypeProperty') { + if (property.variance && property.variance.kind === 'plus') { + shouldThrow = true; + } else { + shouldThrow = false; + break; + } + } else if (property.type === 'ObjectTypeSpreadProperty') { + hasSpread = true; + } + } + if (hasSpread === true && shouldThrow === true) { + context.report({ + messageId: 'readonlySpread', + node: node.right, + }); + } + } + }, + }; +}; + +export default { + create, + meta, +}; diff --git a/tests/rules/assertions/useReadOnlySpread.js b/tests/rules/assertions/useReadOnlySpread.js new file mode 100644 index 0000000..815940d --- /dev/null +++ b/tests/rules/assertions/useReadOnlySpread.js @@ -0,0 +1,76 @@ +export default { + invalid: [ + { + code: `type INode = {||}; +type Identifier = {| + ...INode, + +aaa: string, +|};`, + errors: [{ + message: 'Flow type with spread property and all readonly properties should be wrapped in \'$ReadOnly<…>\' to prevent accidental loss of readonly-ness.', + }], + }, + { + code: `type INode = {||}; +type Identifier = {| + ...INode, + +aaa: string, + +bbb: string, +|};`, + errors: [{ + message: 'Flow type with spread property and all readonly properties should be wrapped in \'$ReadOnly<…>\' to prevent accidental loss of readonly-ness.', + }], + }, + ], + + valid: [ + // Object with spread operator: + { + code: `type INode = {||}; +type Identifier = {| + ...INode, + name: string, +|};`, + }, + { + code: `type INode = {||}; +type Identifier = {| + ...INode, + name: string, // writable on purpose + +surname: string, +|};`, + }, + + // Object without spread operator: + { + code: `type Identifier = {| + +name: string, +|};`, + }, + + // Read-only object with spread: + { + code: `type INode = {||}; +type Identifier = $ReadOnly<{| + ...INode, + +name: string, +|}>;`, + }, + { + code: `type INode = {||}; +type Identifier = $ReadOnly<{| + ...INode, + name: string, // writable on purpose +|}>;`, + }, + + // Write-only object with spread: + { + code: `type INode = {||}; +type Identifier = $ReadOnly<{| + ...INode, + -name: string, +|}>;`, + }, + ], +}; diff --git a/tests/rules/index.js b/tests/rules/index.js index 7b004c9..0383654 100644 --- a/tests/rules/index.js +++ b/tests/rules/index.js @@ -51,6 +51,7 @@ const reportingRules = [ 'type-import-style', 'union-intersection-spacing', 'use-flow-type', + 'use-read-only-spread', 'valid-syntax', ];