search

dsherret / ts-type-info

TypeScript AST and code generator [Deprecated]
MIT License
94 stars 8 forks source link

Multiline/multiple variable declarations yield only first declaration #221

Closed michaelneu closed 7 years ago

michaelneu commented 7 years ago

First things first: thank you for this awesome project!

As the title says, I'm trying to get multiline-variable-declarations parsed, but the info only contains the first one of each multiline-statement.

file-to-parse.d.ts

declare const a: boolean,
              b: number;

declare const c: string,
              d: any;

main.ts

import { getInfoFromFiles } from "ts-type-info";

const filename = "file-to-parse.d.ts";

const info = getInfoFromFiles([filename]),
      myFiles = info.files.filter((file) => file.fileName === filename);

if (myFiles.length == 1) {
    const myFile = myFiles[0];

    for (const variable of myFile.variables) {
        console.log(`declared "${variable.name}" as "${variable.type.text}"`);
    }
}

Output

declared "a" as "boolean"
declared "c" as "string"

As I'm trying to migrate my current raw-compiler-API-implementation to ts-type-info, I can say that the API does yield all statements, but I didn't quite fully grasp how all components work together in ts-type-info yet.

However, a quick F12-journey told me that this might fix the issue:

diff --git a/TsNode-original.ts b/TsNode-possible-fix.ts
index 881702e..91b354f 100644
--- a/TsNode-original.ts
+++ b/TsNode-possible-fix.ts
@@ -660,14 +660,14 @@ export class TsNode extends TsSourceFileChild {
             if (this.isNotDisallowedNode(node)) {
                 if (node.kind === ts.SyntaxKind.VariableStatement) {
                     const declarationList = (node as ts.VariableStatement).declarationList;
-                    node = declarationList.declarations[0];
-
-                    if (declarationList.declarations.length > 1) {
-                        Logger.warn(`Unknown situation where declaration list was greater than 1 for ${node.getText(this.sourceFile)}`);
+                    
+                    for (const node of declarationList.declarations) {
+                        callback(this.createNode(node));
                     }
                 }
-
-                callback(this.createNode(node));
+                else {
+                    callback(this.createNode(node));
+                }
             }
         });
     }

Output with dirty fix

declared "a" as "boolean"
declared "b" as "number"
declared "c" as "string"
declared "d" as "any"

If you want, I can invest some more time and take a deeper look into the code base, to create a more qualified and tested PR.

dsherret commented 7 years ago

Hey Michael, I'm glad this project has been helpful for you and thanks for reporting this issue! I forgot to consider that scenario. I believe your fix is correct.

If you would like, you can create a PR for this on the dev branch and I'll kick off a release with the fix. I believe you only need to change that file and then for a test add some code with a variable declaration list to this file, then update the structure in that test file accordingly. The other tests will catch if anything else broke because of the change (which I doubt).

Otherwise, I will get around to it sometime this month. I'm currently actively working on this project making some major changes on my local machine.

michaelneu commented 7 years ago

Hi David,

I forked the repo and added the both the fix and test cases, however I ran into some issues while trying to npm test:

[22:55:05] Using gulpfile ~/ts-type-info/gulpfile.js
[22:55:05] Starting 'tslint'...
[22:55:16] Finished 'tslint' after 11 s
[22:55:24] Using gulpfile ~/ts-type-info/gulpfile.js
[22:55:24] Starting 'clean-scripts'...
[22:55:24] Finished 'clean-scripts' after 231 ms
[22:55:24] Starting 'typescript'...
src/binders/ts/file/TsReExportBinder.ts(28,60): error TS7006: Parameter 'node' implicitly has an 'any' type.
src/compiler/TsNode.ts(87,39): error TS2339: Property 'Private' does not exist on type 'typeof NodeFlags'.
src/compiler/TsNode.ts(90,44): error TS2339: Property 'Protected' does not exist on type 'typeof NodeFlags'.
src/compiler/TsNode.ts(93,44): error TS2339: Property 'Public' does not exist on type 'typeof NodeFlags'.
src/compiler/TsNode.ts(198,16): error TS2349: Cannot invoke an expression whose type lacks a call signature. Type '{ <U>(this: [ImportSpecifier, ImportSpecifier, ImportSpecifier, ImportSpecifier, ImportSpecifier]...' has no compatible call signatures.
src/compiler/TsNode.ts(198,50): error TS7006: Parameter 'e' implicitly has an 'any' type.
src/compiler/TsNode.ts(239,39): error TS2339: Property 'Private' does not exist on type 'typeof NodeFlags'.
src/compiler/TsNode.ts(242,44): error TS2339: Property 'Protected' does not exist on type 'typeof NodeFlags'.
src/compiler/TsNode.ts(691,50): error TS2339: Property 'Ambient' does not exist on type 'typeof NodeFlags'.
src/compiler/TsType.ts(120,48): error TS2339: Property 'ThisType' does not exist on type 'typeof TypeFlags'.
src/compiler/TsType.ts(124,48): error TS2339: Property 'Tuple' does not exist on type 'typeof TypeFlags'.
src/compiler/TsType.ts(150,48): error TS2339: Property 'Anonymous' does not exist on type 'typeof TypeFlags'.
src/compiler/TsType.ts(154,48): error TS2339: Property 'Reference' does not exist on type 'typeof TypeFlags'.
src/compiler/TsType.ts(158,48): error TS2339: Property 'Class' does not exist on type 'typeof TypeFlags'.
src/compiler/TsType.ts(162,48): error TS2339: Property 'Interface' does not exist on type 'typeof TypeFlags'.
src/main.ts(60,12): error TS2454: Variable 'fileDefinition' is used before being assigned.
[22:55:29] TypeScript: 16 semantic errors
[22:55:29] TypeScript: emit succeeded (with errors)
[22:55:29] Finished 'typescript' after 4.78 s
[22:55:29] Starting 'pre-test'...
[22:55:30] Finished 'pre-test' after 627 ms
[22:55:30] Starting 'test'...

events.js:160
      throw er; // Unhandled 'error' event
      ^
TypeError: Cannot read property 'export=' of undefined
    at resolveExternalModuleSymbol (/home/michael/ts-type-info/node_modules/typescript/lib/typescript.js:25449:86)
    at getExportsForModule (/home/michael/ts-type-info/node_modules/typescript/lib/typescript.js:25508:28)
    at getExportsOfModule (/home/michael/ts-type-info/node_modules/typescript/lib/typescript.js:25479:70)
    at Object.getExportsOfModuleAsArray [as getExportsOfModule] (/home/michael/ts-type-info/node_modules/typescript/lib/typescript.js:25466:35)
    at TsTypeChecker.getExportsOfModule (/home/michael/ts-type-info/dist/compiler/utils/TsTypeChecker.js:9:5381)
    at TsSymbol.getExportSymbolsOfModuleByName (/home/michael/ts-type-info/dist/compiler/TsSymbol.js:9:6722)
    at TsImportBinder.getStarImports (/home/michael/ts-type-info/dist/binders/ts/file/TsImportBinder.js:9:3893)
    at TsImportBinder.ImportBinder.bind (/home/michael/ts-type-info/dist/binders/base/file/ImportBinder.js:9:1011)
    at /home/michael/ts-type-info/dist/factories/TsFactory.js:9:17608
    at Array.forEach (native)

The error seems to be a somewhat general error, as I also ran into it on the raw origin/dev-branch.

dsherret commented 7 years ago

Hmmm strange. That's not happening on the build server. I'll have to take a look in a bit, but I have a feeling that it's compiling that with 2.1. Right now the project is still on 2.0—not 2.1—but gulp typescript should be building with the package in the node_modules folder so that's weird.

That fileDefinition error is also strange... they must not have thought about try finally statements.

dsherret commented 7 years ago

Hey @michaelneu, it was indeed because it was using 2.1. They made a bunch of changes to the API as usual.

Anyway, I just went through and fixed all the errors for 2.1 in the latest commit on the dev branch. It should be good now. Sorry about that! If you would like, I'll take a look at this issue more on the weekend, but if you would like to contribute then please submit a PR.

michaelneu commented 7 years ago

Thanks for your fix! I did have tsc 2.1.4 globally installed, but because of gulp I didn't think it'd take the global one either.

I ran my new tests on your dev-branch and they went through - except for one. I think there's an error when detecting the implicit type of a const variable (I'll create a new issue to not create a mess here). I modified the failing test case, and created a PR.

dsherret commented 7 years ago

Thanks again for the fix! Closing.