From 95d3ac766944095f4411dff904a3666453294d5f Mon Sep 17 00:00:00 2001 From: Alexander Pavlyuk Date: Mon, 17 Apr 2023 17:45:32 +0300 Subject: [PATCH] Issue #11407: unproper use of 'instanceof'. 'finally' and 'getters/setters' are not reporeted as problems any more. Change-Id: I3d8c2d4b2d0756614011532c8736ce4372aca877 Signed-off-by: Alexander Pavlyuk --- .../{finally_clause.ts => instanceof.ts} | 43 ++++++------------ ...ts.relax.json => instanceof.ts.relax.json} | 34 ++++++-------- ....strict.json => instanceof.ts.strict.json} | 44 ++++++------------ .../linter/type_declarations.ts.relax.json | 20 --------- .../linter/type_declarations.ts.strict.json | 20 --------- migrator/typescript/src/linter/Problems.ts | 5 ++- .../typescript/src/linter/TypeScriptLinter.ts | 45 ++++++++++--------- migrator/typescript/src/linter/Utils.ts | 19 +++++++- 8 files changed, 84 insertions(+), 146 deletions(-) rename migrator/test/linter/{finally_clause.ts => instanceof.ts} (58%) rename migrator/test/linter/{finally_clause.ts.relax.json => instanceof.ts.relax.json} (61%) rename migrator/test/linter/{finally_clause.ts.strict.json => instanceof.ts.strict.json} (52%) diff --git a/migrator/test/linter/finally_clause.ts b/migrator/test/linter/instanceof.ts similarity index 58% rename from migrator/test/linter/finally_clause.ts rename to migrator/test/linter/instanceof.ts index 4a7129acb..0ebd7b4f6 100644 --- a/migrator/test/linter/finally_clause.ts +++ b/migrator/test/linter/instanceof.ts @@ -13,35 +13,18 @@ * limitations under the License. */ -try { - if (true) - throw new Error("Error"); -} -catch(e) { - console.log(e); -} -finally { - console.log("'finally' of the first 'try'"); -} +class X {} -try { - if (true) - throw "Just throw"; -} -catch(e) { - console.log(e); -} -finally { - console.log("'finally' of the second 'try'"); -} +let a = (new X()) instanceof Object // true +let b = (new X()) instanceof X // true -try { - if (true) - throw "Catch without explicit type"; -} -catch(e) { - console.log(e); -} -finally { - console.log("'finally' of the third 'try'"); -} +// left operand is a type: +let c = X instanceof Object // Compile-time error +let d = X instanceof X // Compile-time error + +// left operand may be of any reference type, like number +let e = (5.0 as Number) instanceof Number // false + +let f = (X | String) instanceof Object; + +let g = 3 instanceof Number; diff --git a/migrator/test/linter/finally_clause.ts.relax.json b/migrator/test/linter/instanceof.ts.relax.json similarity index 61% rename from migrator/test/linter/finally_clause.ts.relax.json rename to migrator/test/linter/instanceof.ts.relax.json index 6a6094e80..77ad46c0a 100644 --- a/migrator/test/linter/finally_clause.ts.relax.json +++ b/migrator/test/linter/instanceof.ts.relax.json @@ -1,46 +1,38 @@ { "copyright": [ - "Copyright (c) 2022-2023 Huawei Device Co., Ltd.", + "Copyright (c) 2023-2023 Huawei Device Co., Ltd.", "Licensed under the Apache License, Version 2.0 (the 'License');", "you may not use this file except in compliance with the License.", "You may obtain a copy of the License at", + "", "http://www.apache.org/licenses/LICENSE-2.0", + "", "Unless required by applicable law or agreed to in writing, software", "distributed under the License is distributed on an 'AS IS' BASIS,", "WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.", "See the License for the specific language governing permissions and", "limitations under the License." - ], + ], "nodes": [ { - "line": 16, - "column": 1, - "problem": "TopLevelStmt" - }, - { - "line": 23, + "line": 22, "column": 9, - "problem": "FinallyKeyword" + "problem": "InstanceofUnsupported" }, { - "line": 27, - "column": 1, - "problem": "TopLevelStmt" - }, - { - "line": 34, + "line": 23, "column": 9, - "problem": "FinallyKeyword" + "problem": "InstanceofUnsupported" }, { - "line": 38, - "column": 1, - "problem": "TopLevelStmt" + "line": 28, + "column": 10, + "problem": "BitOpWithWrongType" }, { - "line": 45, + "line": 30, "column": 9, - "problem": "FinallyKeyword" + "problem": "InstanceofUnsupported" } ] } \ No newline at end of file diff --git a/migrator/test/linter/finally_clause.ts.strict.json b/migrator/test/linter/instanceof.ts.strict.json similarity index 52% rename from migrator/test/linter/finally_clause.ts.strict.json rename to migrator/test/linter/instanceof.ts.strict.json index 31276b3e8..77ad46c0a 100644 --- a/migrator/test/linter/finally_clause.ts.strict.json +++ b/migrator/test/linter/instanceof.ts.strict.json @@ -1,56 +1,38 @@ { "copyright": [ - "Copyright (c) 2022-2023 Huawei Device Co., Ltd.", + "Copyright (c) 2023-2023 Huawei Device Co., Ltd.", "Licensed under the Apache License, Version 2.0 (the 'License');", "you may not use this file except in compliance with the License.", "You may obtain a copy of the License at", + "", "http://www.apache.org/licenses/LICENSE-2.0", + "", "Unless required by applicable law or agreed to in writing, software", "distributed under the License is distributed on an 'AS IS' BASIS,", "WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.", "See the License for the specific language governing permissions and", "limitations under the License." - ], + ], "nodes": [ { - "line": 16, - "column": 1, - "problem": "TopLevelStmt" - }, - { - "line": 23, + "line": 22, "column": 9, - "problem": "FinallyKeyword" - }, - { - "line": 27, - "column": 1, - "problem": "TopLevelStmt" + "problem": "InstanceofUnsupported" }, { - "line": 34, + "line": 23, "column": 9, - "problem": "FinallyKeyword" - }, - { - "line": 29, - "column": 5, - "problem": "ThrowStatement" + "problem": "InstanceofUnsupported" }, { - "line": 38, - "column": 1, - "problem": "TopLevelStmt" + "line": 28, + "column": 10, + "problem": "BitOpWithWrongType" }, { - "line": 45, + "line": 30, "column": 9, - "problem": "FinallyKeyword" - }, - { - "line": 40, - "column": 5, - "problem": "ThrowStatement" + "problem": "InstanceofUnsupported" } ] } \ No newline at end of file diff --git a/migrator/test/linter/type_declarations.ts.relax.json b/migrator/test/linter/type_declarations.ts.relax.json index d14b2447f..2e20e96be 100644 --- a/migrator/test/linter/type_declarations.ts.relax.json +++ b/migrator/test/linter/type_declarations.ts.relax.json @@ -69,31 +69,11 @@ "column": 20, "problem": "AnyType" }, - { - "line": 44, - "column": 5, - "problem": "GetterSetter" - }, - { - "line": 45, - "column": 5, - "problem": "GetterSetter" - }, { "line": 49, "column": 1, "problem": "TopLevelStmt" }, - { - "line": 49, - "column": 1, - "problem": "GetterSetter" - }, - { - "line": 50, - "column": 10, - "problem": "GetterSetter" - }, { "line": 52, "column": 1, diff --git a/migrator/test/linter/type_declarations.ts.strict.json b/migrator/test/linter/type_declarations.ts.strict.json index 2a7299713..52b0c6112 100644 --- a/migrator/test/linter/type_declarations.ts.strict.json +++ b/migrator/test/linter/type_declarations.ts.strict.json @@ -69,31 +69,11 @@ "column": 20, "problem": "AnyType" }, - { - "line": 44, - "column": 5, - "problem": "GetterSetter" - }, - { - "line": 45, - "column": 5, - "problem": "GetterSetter" - }, { "line": 49, "column": 1, "problem": "TopLevelStmt" }, - { - "line": 49, - "column": 1, - "problem": "GetterSetter" - }, - { - "line": 50, - "column": 10, - "problem": "GetterSetter" - }, { "line": 52, "column": 1, diff --git a/migrator/typescript/src/linter/Problems.ts b/migrator/typescript/src/linter/Problems.ts index a13318b89..1fab73ee8 100644 --- a/migrator/typescript/src/linter/Problems.ts +++ b/migrator/typescript/src/linter/Problems.ts @@ -45,7 +45,7 @@ export enum NodeType { SpreadOperator, KeyOfOperator, ImportFromPath, - GetterSetter, + //GetterSetter, FunctionExpression, TypeParameterWithDefaultValue, @@ -101,7 +101,6 @@ export enum NodeType { ImplementsClass, MultipleStaticBlocks, - FinallyKeyword, //Decorators, // It's not a problem and counted temporary just to have statistic of decorators use. ThisType, InferType, @@ -128,6 +127,8 @@ export enum NodeType { InterfaceOptionalProp, ParameterProperties, + InstanceofUnsupported, + LAST_NODE_TYPE // this should always be last enum` } diff --git a/migrator/typescript/src/linter/TypeScriptLinter.ts b/migrator/typescript/src/linter/TypeScriptLinter.ts index 077688837..29f52411f 100644 --- a/migrator/typescript/src/linter/TypeScriptLinter.ts +++ b/migrator/typescript/src/linter/TypeScriptLinter.ts @@ -111,7 +111,7 @@ export class TypeScriptLinter { TypeScriptLinter.nodeDescription[NodeType.SpreadOperator] = "'spread' operations "; TypeScriptLinter.nodeDescription[NodeType.KeyOfOperator] = "'keyof' operations "; TypeScriptLinter.nodeDescription[NodeType.ImportFromPath] = "imports from path nodes "; - TypeScriptLinter.nodeDescription[NodeType.GetterSetter] = "getters/setters nodes "; + //TypeScriptLinter.nodeDescription[NodeType.GetterSetter] = "getters/setters nodes "; TypeScriptLinter.nodeDescription[NodeType.FunctionExpression] = "function expression nodes "; TypeScriptLinter.nodeDescription[NodeType.TypeParameterWithDefaultValue] = "type parameters with default value "; @@ -166,7 +166,6 @@ export class TypeScriptLinter { TypeScriptLinter.nodeDescription[NodeType.ImplementsClass] = "'implements' a class "; TypeScriptLinter.nodeDescription[NodeType.MultipleStaticBlocks] = "Multiple static blocks "; - TypeScriptLinter.nodeDescription[NodeType.FinallyKeyword] = "'finally' "; //TypeScriptLinter.nodeDescription[NodeType.Decorators] = "Decorators "; // It's not a problem and counted temporary just to have statistic of decorators use. TypeScriptLinter.nodeDescription[NodeType.ThisType] = "'this' type "; TypeScriptLinter.nodeDescription[NodeType.InferType] = "Infer type "; @@ -192,6 +191,8 @@ export class TypeScriptLinter { TypeScriptLinter.nodeDescription[NodeType.StringLiteralType] = "string literal type "; TypeScriptLinter.nodeDescription[NodeType.InterfaceOptionalProp] = "Interface optional property "; TypeScriptLinter.nodeDescription[NodeType.ParameterProperties] = "Parameter properties in constructor "; + + TypeScriptLinter.nodeDescription[NodeType.InstanceofUnsupported] = "Left-hand side of 'instanceof' is wrong "; } static { @@ -712,10 +713,10 @@ export class TypeScriptLinter { } break; - case ts.SyntaxKind.GetAccessor: - case ts.SyntaxKind.SetAccessor: - self.incrementCounters(node, NodeType.GetterSetter); - break; + // case ts.SyntaxKind.GetAccessor: + // case ts.SyntaxKind.SetAccessor: + // self.incrementCounters(node, NodeType.GetterSetter); + // break; case ts.SyntaxKind.LiteralType: let tsLiteralType = node as ts.LiteralTypeNode; @@ -726,13 +727,13 @@ export class TypeScriptLinter { case ts.SyntaxKind.PropertyAccessExpression: let propertyAccessNode = node as ts.PropertyAccessExpression; - const symbol = TypeScriptLinter.tsTypeChecker.getSymbolAtLocation(propertyAccessNode.name); - if (symbol !== null && symbol !== undefined) { - let flags = symbol.getFlags(); - if (flags & (ts.SymbolFlags.GetAccessor | ts.SymbolFlags.SetAccessor) ) { - self.incrementCounters(node, NodeType.GetterSetter); - } - } + // const symbol = TypeScriptLinter.tsTypeChecker.getSymbolAtLocation(propertyAccessNode.name); + // if (symbol !== null && symbol !== undefined) { + // let flags = symbol.getFlags(); + // if (flags & (ts.SymbolFlags.GetAccessor | ts.SymbolFlags.SetAccessor) ) { + // self.incrementCounters(node, NodeType.GetterSetter); + // } + // } if (self.isObjectRuntimeCheck(propertyAccessNode)) self.incrementCounters(node, NodeType.ObjectRuntimeCheck); @@ -930,7 +931,16 @@ export class TypeScriptLinter { } } self.incrementCounters(node, NodeType.CommaOperator); - + } + else if (tsBinaryExpr.operatorToken.kind === ts.SyntaxKind.InstanceOfKeyword) { + let leftExpr = Utils.unwrapParenthesizedExpression(tsBinaryExpr.left); + let leftSymbol = TypeScriptLinter.tsTypeChecker.getSymbolAtLocation(leftExpr); + let leftType = TypeScriptLinter.tsTypeChecker.getTypeAtLocation(leftExpr); + // In STS, the left-hand side expression may be of any reference type, otherwise a compile-time error occurs. + // In addition, the left operand in STS cannot be a type. + if (ts.isTypeNode(leftExpr) || !Utils.isRefereceType(leftOperandType) || Utils.isTypeSymbol(leftSymbol)) { + self.incrementCounters(node, NodeType.InstanceofUnsupported); + } } break; @@ -1120,13 +1130,6 @@ export class TypeScriptLinter { break; - case ts.SyntaxKind.TryStatement: // It's not a problem and counted temporary just to have statistic of 'finally' use. - let tsTryStmt = node as ts.TryStatement; - if (tsTryStmt.finallyBlock) - self.incrementCounters(tsTryStmt.finallyBlock, NodeType.FinallyKeyword); - - break; - case ts.SyntaxKind.PrivateIdentifier: self.incrementCounters(node, NodeType.PrivateIdentifier); break; diff --git a/migrator/typescript/src/linter/Utils.ts b/migrator/typescript/src/linter/Utils.ts index eb2b202a2..f3b2b96c5 100644 --- a/migrator/typescript/src/linter/Utils.ts +++ b/migrator/typescript/src/linter/Utils.ts @@ -57,6 +57,24 @@ export function isArrayNotTupleType(tsType: ts.TypeNode): boolean { return false; } +export function isRefereceType(tsType: ts.Type): boolean { + let f = tsType.getFlags(); + + return (f & ts.TypeFlags.InstantiableNonPrimitive) != 0 + || (f & ts.TypeFlags.Object) != 0 + || (f & ts.TypeFlags.Boolean) != 0 + || (f & ts.TypeFlags.Enum) != 0 + || (f & ts.TypeFlags.NonPrimitive) != 0 + || (f & ts.TypeFlags.Number) != 0 + || (f & ts.TypeFlags.String) != 0; + //|| (f & ts.TypeFlags.C) + //|| (f & ts.TypeFlags.Instantiable) != 0 +} + +export function isTypeSymbol(symbol: ts.Symbol) { + return symbol && symbol.flags && (symbol.flags & ts.SymbolFlags.Class || symbol.flags & ts.SymbolFlags.Interface); +} + export function isNumberType(tsType: ts.Type): boolean { return (tsType.getFlags() & (ts.TypeFlags.NumberLike)) != 0; } @@ -69,7 +87,6 @@ export function isStringType(tsType: ts.Type): boolean { return (tsType.getFlags() & (ts.TypeFlags.StringLike)) != 0; } - export function unwrapParenthesizedType(tsType: ts.TypeNode): ts.TypeNode { while (ts.isParenthesizedTypeNode(tsType)) { tsType = tsType.type; -- Gitee