fuhrmanator / FamixTypeScriptImporter

MIT License
2 stars 1 forks source link

Need to recognize ambient modules as Famix.Module #58

Closed fuhrmanator closed 2 months ago

fuhrmanator commented 3 months ago

EDIT: This bug is related to ambient modules, which are used in declaration files (.d.ts). They're a special case that is not handled properly.


The importer finds the following file (in the log210-jeu-de-des-express-typescript` project) and it's identified as a ScriptElement (according to the debugger), whereas I think it should be a Module (because it has import/export statements in a namespace).

// Type definitions for express-flash-plus (modified from connect-flash by C. Fuhrman)
// Project: https://github.com/jaredhanson/connect-flash
// Definitions by: Andreas Gassmann <https://github.com/AndreasGassmann>
//                 Drew Lemmy <https://github.com/Lemmmy>
// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
// TypeScript Version: 2.3

/// <reference types="express" />

declare namespace Express {
    export interface Request {
        flash(): { [key: string]: string[] };
        flash(message: string): string[];
        flash(type: string, message: string[] | string): number;
        flash(type: string, format: string, ...args: any[]): number;
    }
}

declare module "express-flash-plus" {
    import express = require('express');
    interface IConnectFlashOptions {
        unsafe?: boolean | undefined;
    }
    function e(options?: IConnectFlashOptions): express.RequestHandler;
    export = e;
}
fuhrmanator commented 3 months ago

Further debugging leads to this module "express-flash-plus" being modeled as a Namespace rather than a Module, which seems odd. The problem is that an ImportClause needs to be created relative to a Module. The code crashes when fmxImportClause.setImportingEntity(fmxImporter); runs and fmxImporter is a Namespace. I suspect the FQN for the Module is colliding with the Namespace. The FQNFunctions.getFQN maybe needs to put the entity type (Namespace, Module) inside to avoid this.

        const importerFullyQualifiedName = FQNFunctions.getFQN(importer);
        const fmxImporter = this.famixRep.getFamixEntityByFullyQualifiedName(importerFullyQualifiedName) as Famix.Module;
        fmxImportClause.setImportingEntity(fmxImporter);
fuhrmanator commented 3 months ago

I tried reverse lookup with fmxElementObjectMap and it returns a Famix.Namespace rather than Famix.Module. A dump of the keys() for that structure reveals NO modules. AST Viewer shows clearly a ModuleDeclaration in TSMorph.

fuhrmanator commented 3 months ago

Possibly need to have an isAmbient property for Famix.Module.

fuhrmanator commented 3 months ago

Proposal: Drop Famix.Namespace and use Famix.Module (only) with isAmbient, isModule, isNamespace.

In TypeScript's AST there is no difference (they're all Module).