Skip to content

Commit

Permalink
metro-react-native-interop-tools modified the error structure
Browse files Browse the repository at this point in the history
Summary:
I changed the error structure from a string to an object with message (a human readable string) and the typeLoc(source location of the code).
This structure is more useful if I'm creating more helpers that are handling the errors for different types.
When I will add the pointing to the code, the final error for this code
```
foo.js old version:
23| type T = string;

foo.js new version:
23| type T = ?string;
```
will be something like this:
```
Error: cannot change string to nullable string because it will break the native code.

path/to/foo.js:

23| type T = ?string;
    ^^^^^^^^^^^^^^^^^
```
Instead of appending the code source in every helper (basicError, literalError, etc.), I store the source location to process all the array of errors in an additional function.
The new parameter in compareTypeAnnotation(newVersion) is used to give the context from the previous recursion. This is useful when I have the return type of the function. In that case, the left is switched with the right for comparison because I'm not calling the native code from js, instead the native returns something to js.
```() => boolean? -> () => boolean``` this will cause an incompatibility because nullable boolean is changed to boolean in the returned type

Reviewed By: motiz88

Differential Revision: D39271900

fbshipit-source-id: e7ed4d5a99b48814fbae24a1773f25843600500e
  • Loading branch information
mirtleonard authored and facebook-github-bot committed Sep 8, 2022
1 parent 93d1a53 commit 2995129
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ exports[`comparing basic types 5`] = `
right-type:
number
output:
Error: NumberTypeAnnotation cannot be assigned to BooleanTypeAnnotation"
Error: cannot change boolean to number because it will break the native code.
"
`;

exports[`comparing basic types 6`] = `
Expand All @@ -67,7 +68,8 @@ exports[`comparing basic types 7`] = `
right-type:
'b'
output:
Error: StringLiteralTypeAnnotation cannot be assigned to StringLiteralTypeAnnotation"
Error: cannot change string literal to string literal because it will break the native code.
"
`;

exports[`comparing basic types 8`] = `
Expand All @@ -87,7 +89,8 @@ exports[`comparing basic types 9`] = `
right-type:
8
output:
Error: NumberLiteralTypeAnnotation cannot be assigned to NumberLiteralTypeAnnotation"
Error: cannot change number literal to number literal because it will break the native code.
"
`;

exports[`comparing basic types 10`] = `
Expand All @@ -97,7 +100,8 @@ exports[`comparing basic types 10`] = `
right-type:
number
output:
Error: NumberTypeAnnotation cannot be assigned to NumberLiteralTypeAnnotation"
Error: cannot change number literal to number because it will break the native code.
"
`;

exports[`comparing basic types 11`] = `
Expand Down Expand Up @@ -127,7 +131,8 @@ exports[`comparing basic types 13`] = `
right-type:
string
output:
Error: StringTypeAnnotation cannot be assigned to StringLiteralTypeAnnotation"
Error: cannot change string literal to string because it will break the native code.
"
`;

exports[`comparing basic types 14`] = `
Expand All @@ -147,7 +152,8 @@ exports[`comparing basic types 15`] = `
right-type:
false
output:
Error: BooleanLiteralTypeAnnotation cannot be assigned to BooleanLiteralTypeAnnotation"
Error: cannot change boolean literal to boolean literal because it will break the native code.
"
`;

exports[`comparing basic types 16`] = `
Expand All @@ -157,7 +163,8 @@ exports[`comparing basic types 16`] = `
right-type:
?string
output:
Error: NullableTypeAnnotation cannot be assigned to StringTypeAnnotation"
Error: cannot change string to nullable string because it will break the native code.
"
`;

exports[`comparing basic types 17`] = `
Expand Down Expand Up @@ -207,7 +214,8 @@ exports[`comparing basic types 21`] = `
right-type:
?number
output:
Error: NumberTypeAnnotation cannot be assigned to StringTypeAnnotation"
Error: cannot change string to number because it will break the native code.
"
`;

exports[`comparing basic types 22`] = `
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,21 @@ test.each([
['?string', 'null'],
['?boolean', 'void'],
])('comparing basic types', (left, right) => {
let result = compareTypeAnnotation(
const result = compareTypeAnnotation(
getTypeFromCode(left),
getTypeFromCode(right),
).join('\n\t\t\t');
result = result !== '' ? result : 'no errors';
'right',
);
let messages: string = '';
result.forEach(error => {
messages = messages + error.message + '\n\t\t';
});
messages = messages === '' ? 'no errors' : messages;
expect(`
left-type:
${left}
right-type:
${right}
output:
${result}`).toMatchSnapshot();
${messages}`).toMatchSnapshot();
});
108 changes: 70 additions & 38 deletions packages/metro-react-native-interop-tools/src/type-comparison.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,53 @@
* @format
*/

import type {AnyTypeAnnotation} from './type-annotation.js';
import type {
AnyTypeAnnotation,
NullableTypeAnnotation,
} from './type-annotation.js';
import type {SourceLocation} from '@babel/types';

function makeError(
status: 'incompatible-types' | 'unknown-types',
left: AnyTypeAnnotation,
right: AnyTypeAnnotation,
): string {
if (status === 'incompatible-types') {
return `Error: ${right.type} cannot be assigned to ${left.type}`;
type ComparisonResult = $ReadOnly<{
message: string,
typeLoc: ?SourceLocation,
}>;

function removeNullable(annotation: NullableTypeAnnotation) {
if (annotation.typeAnnotation.type === 'NullableTypeAnnotation') {
return removeNullable(annotation.typeAnnotation);
}
return annotation.typeAnnotation;
}

function getSimplifiedType(annotation: AnyTypeAnnotation): string {
switch (annotation.type) {
case 'BooleanTypeAnnotation':
return 'boolean';
case 'StringTypeAnnotation':
return 'string';
case 'NumberTypeAnnotation':
return 'number';
case 'VoidTypeAnnotation':
return 'void';
case 'StringLiteralTypeAnnotation':
return 'string literal';
case 'NumberLiteralTypeAnnotation':
return 'number literal';
case 'BooleanLiteralTypeAnnotation':
return 'boolean literal';
case 'NullLiteralTypeAnnotation':
return 'null';
case 'NullableTypeAnnotation':
return `nullable ${getSimplifiedType(removeNullable(annotation))}`;
}
throw new Error(left.type + ' is not supported');
throw new Error(annotation.type + ' is not supported');
}

export function compareTypeAnnotation(
left: AnyTypeAnnotation,
right: AnyTypeAnnotation,
): $ReadOnlyArray<string> {
newVersion: 'left' | 'right',
): Array<ComparisonResult> {
switch (left.type) {
case 'BooleanTypeAnnotation':
if (
Expand All @@ -34,69 +64,71 @@ export function compareTypeAnnotation(
) {
return [];
}
return [makeError('incompatible-types', left, right)];
return [basicError(left, right, newVersion)];
case 'NumberTypeAnnotation':
if (
left.type === right.type ||
right.type === 'NumberLiteralTypeAnnotation'
) {
return [];
}
return [makeError('incompatible-types', left, right)];
return [basicError(left, right, newVersion)];
case 'StringTypeAnnotation':
if (
left.type === right.type ||
right.type === 'StringLiteralTypeAnnotation'
) {
return [];
}
return [makeError('incompatible-types', left, right)];
return [basicError(left, right, newVersion)];
case 'VoidTypeAnnotation':
if (right.type === 'VoidTypeAnnotation') {
return [];
}
return [makeError('incompatible-types', left, right)];
return [basicError(left, right, newVersion)];
case 'StringLiteralTypeAnnotation':
if (
right.type === 'StringLiteralTypeAnnotation' &&
right.value === left.value
) {
return [];
}
return [makeError('incompatible-types', left, right)];
case 'NumberLiteralTypeAnnotation':
if (
right.type === 'NumberLiteralTypeAnnotation' &&
right.value === left.value
) {
return [];
}
return [makeError('incompatible-types', left, right)];
case 'BooleanLiteralTypeAnnotation':
if (
right.type === 'BooleanLiteralTypeAnnotation' &&
right.value === left.value
) {
case 'NumberLiteralTypeAnnotation':
if (right.type === left.type && right.value === left.value) {
return [];
}
return [makeError('incompatible-types', left, right)];
return [basicError(left, right, newVersion)];
case 'NullLiteralTypeAnnotation':
if (right.type !== 'NullLiteralTypeAnnotation') {
return [makeError('incompatible-types', left, right)];
return [basicError(left, right, newVersion)];
}
return [];
case 'NullableTypeAnnotation':
if (right.type === 'NullableTypeAnnotation') {
return compareTypeAnnotation(left.typeAnnotation, right.typeAnnotation);
return compareTypeAnnotation(
left.typeAnnotation,
right.typeAnnotation,
newVersion,
);
}
if (
right.type === 'NullLiteralTypeAnnotation' ||
right.type === 'VoidTypeAnnotation'
) {
return [];
}
return compareTypeAnnotation(left.typeAnnotation, right);
return compareTypeAnnotation(left.typeAnnotation, right, newVersion);
default:
return [makeError('unknown-types', left, right)];
throw new Error(left.type + 'not supported');
}
}

function basicError(
left: AnyTypeAnnotation,
right: AnyTypeAnnotation,
newVersion: 'left' | 'right',
): ComparisonResult {
const newAnnotationType = newVersion === 'right' ? right : left;
const oldAnnotationType = newVersion === 'right' ? left : right;
const newType = getSimplifiedType(newAnnotationType);
const oldType = getSimplifiedType(oldAnnotationType);
return {
message: `Error: cannot change ${oldType} to ${newType} because it will break the native code.`,
typeLoc: newAnnotationType.loc,
};
}

0 comments on commit 2995129

Please sign in to comment.