Skip to content

Commit

Permalink
fix(rosetta): correctly detect arguments typed as any (#3043)
Browse files Browse the repository at this point in the history
Try and resolve the containing function call. If it resolves, we
know the `any` the typechecker is giving us is actually intended to
be an `any`.

If the containing call does not resolve, we'll treat it as an unknown
type of an uncompiling sample, and assume there's a struct there.

Fixes #3029.



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
rix0rrr authored Oct 8, 2021
1 parent 878404d commit 3d2ba15
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 8 deletions.
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/lib/jsii/jsii-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export function isStructType(type: ts.Type) {
);
}

function hasFlag<A extends number>(flags: A, test: A) {
export function hasFlag<A extends number>(flags: A, test: A) {
// tslint:disable-next-line:no-bitwise
return (flags & test) !== 0;
}
Expand Down
29 changes: 27 additions & 2 deletions packages/jsii-rosetta/lib/languages/default.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as ts from 'typescript';

import { isStructInterface, isStructType } from '../jsii/jsii-utils';
import { hasFlag, isStructInterface, isStructType } from '../jsii/jsii-utils';
import { OTree, NO_SYNTAX } from '../o-tree';
import { AstRenderer, AstHandler, nimpl, CommentSyntax } from '../renderer';
import { voidExpressionString } from '../typescript/ast-utils';
Expand Down Expand Up @@ -132,7 +132,21 @@ export abstract class DefaultVisitor<C> implements AstHandler<C> {
public objectLiteralExpression(node: ts.ObjectLiteralExpression, context: AstRenderer<C>): OTree {
const type = typeWithoutUndefinedUnion(context.inferredTypeOfExpression(node));

const isUnknownType = !type || !type.symbol;
let isUnknownType = !type;
if (type && hasFlag(type.flags, ts.TypeFlags.Any)) {
// The type checker by itself won't tell us the difference between an `any` that
// was literally declared as a type in the code, vs an `any` it assumes because it
// can't find a function's type declaration.
//
// Search for the function's declaration and only if we can't find it,
// the type is actually unknown (otherwise it's a literal 'any').
const call = findEnclosingCallExpression(node);
const signature = call ? context.typeChecker.getResolvedSignature(call) : undefined;
if (!signature?.declaration) {
isUnknownType = true;
}
}

const isKnownStruct = type && isStructType(type);

if (isUnknownType) {
Expand Down Expand Up @@ -317,3 +331,14 @@ const UNARY_OPS: { [op in ts.PrefixUnaryOperator]: string } = {
[ts.SyntaxKind.TildeToken]: '~',
[ts.SyntaxKind.ExclamationToken]: '~',
};

function findEnclosingCallExpression(node?: ts.Node): ts.CallLikeExpression | undefined {
while (node) {
if (ts.isCallLikeExpression(node)) {
return node;
}
node = node.parent;
}

return undefined;
}
25 changes: 22 additions & 3 deletions packages/jsii-rosetta/lib/typescript/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,24 @@ export function inferMapElementType(
elements: ts.NodeArray<ts.ObjectLiteralElementLike>,
renderer: AstRenderer<any>,
): ts.Type | undefined {
return typeIfSame(
elements.map((el) => (ts.isPropertyAssignment(el) ? renderer.typeOfExpression(el.initializer) : undefined)),
);
const nodes = elements.map(elementValueNode).filter(isDefined);
const types = nodes.map((x) => renderer.typeOfExpression(x));

return types.every((t) => isSameType(types[0], t)) ? types[0] : undefined;

function elementValueNode(el: ts.ObjectLiteralElementLike): ts.Expression | undefined {
if (ts.isPropertyAssignment(el)) {
return el.initializer;
}
if (ts.isShorthandPropertyAssignment(el)) {
return el.name;
}
return undefined;
}
}

function isSameType(a: ts.Type, b: ts.Type) {
return a.flags === b.flags && a.symbol?.name === b.symbol?.name;
}

function typeIfSame(types: Array<ts.Type | undefined>): ts.Type | undefined {
Expand Down Expand Up @@ -120,3 +135,7 @@ export function arrayElementType(type: ts.Type): ts.Type | undefined {
}
return undefined;
}

function isDefined<A>(x: A): x is NonNullable<A> {
return x !== undefined;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ public void Test(Array _args)
{
}

Test(new Struct { Key = "Value", Also = 1337 });
Test(new Dictionary<string, object> { { "Key", "Value" }, { "also", 1337 } });

Test(new Struct { Key = "Value" }, new Struct { Also = 1337 });
Test(new Dictionary<string, string> { { "Key", "Value" } }, new Dictionary<string, int> { { "also", 1337 } });
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
FunctionThatTakesAnAny(new Dictionary<string, int> {
{ "argument", 5 }
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
functionThatTakesAnAny(Map.of(
"argument", 5));
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
function_that_takes_an_any({
"argument": 5
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/// !hide
function functionThatTakesAnAny(opts: any) { }
/// !show
functionThatTakesAnAny({
argument: 5
});

0 comments on commit 3d2ba15

Please sign in to comment.