Tests shouldn't make code to be considered as used

[t3chguy]

[SimonBrandner]

It would be nice to have further context here

[t3chguy]

A component was added with a test, it was not used anywhere, it was not identified as dead code because it was imported by a test. If we stop using a bit of code and it happens to have a unit test then it will never get identified as dead code

[clarkf]

I played around with this a bit today, and there's definitely some stuff that can be safely removed (e.g. useAccountData, <TooltipButton />. I found about 600 unused lines).

From what I can gather, ts-prune isn't quite flexible enough to deal with the mutli-project structure. I had to drop down and use ts-morph directly:

import { Project, Node, ReferenceFindableNode, SourceFile } from "ts-morph";
import * as path from "node:path";

const root = path.resolve(path.join(__dirname, ".."));

const project = new Project({
    tsConfigFilePath: path.join(root, "tsconfig.json"),
});

const SAFE: Record<string, string[]> = {
    // list symbols that are externally depended upon, e.g.
    "src/rageshake/rageshake": ["init", "flush", "cleanup", "tryInitStorage"],
};

for (const file of project.getSourceFiles()) {
    file.forEachChild((child) => {
        if (Node.isVariableStatement(child)) {
            if (isExported(child)) child.getDeclarations().forEach(checkNode);
        } else if (isExported(child)) checkNode(child);
    });
}

function isExported(node: Node) {
    return Node.isExportable(node) && node.isExported();
}

function checkNode(node: Node) {
    if (Node.isInterfaceDeclaration(node) || Node.isTypeAliasDeclaration(node)) return;
    if (!Node.isReferenceFindable(node)) return;

    const file = node.getSourceFile();
    const filePath = path.relative(root, file.getFilePath());
    const symbName = Node.hasName(node) ? node.getName() : node.getText();

    if (isSafe(node)) {
        return;
    }

    if (isDeadCode(node, file))
        console.log(`[${filePath}:${node.getStartLineNumber()}]: ${symbName} - ${node.getKindName()}`);
}

function isDeadCode(node: ReferenceFindableNode, _file: SourceFile): boolean {
    return node.findReferencesAsNodes().length === 0;
}

function isSafe(node: Node): boolean {
    const file = node.getSourceFile();
    const filePath = path.relative(root, file.getFilePath());
    const symbName = Node.hasName(node) ? node.getName() : node.getText();

    // ts-morph likes to omit extensions, so check against both
    return [filePath, filePath.replace(/\.tsx?$/, "")].some((p) => !!SAFE[p]?.includes(symbName));
}

(Based on the same gist that ts-prune came from).

I've skipped type-level information altogether, since it's elided from the final build, but there's fiddling to be done there too. I could try to add a feature to ts-prune, but it almost seems unnecessary, since element-web is dropping into JS to do this already.

In order for something like this to be realistic, noImplcitAny would need to be true (ref https://github.com/vector-im/element-web/issues/21968 - ambiguous data in, ambiguous data out ;-)). tsconfig.json would also need to be sorted out. I can't remember off the top of my head what best practices are, but I think it involves having separate tsconfigs for src and test/spec.

[t3chguy]

@clarkf thanks for digging into that, I'm working on noImplicitAny right now!

[Johennes]

The work on enabling noImplicitAny has concluded by now.