rokucommunity / bslint

A linter for BrightScript and BrighterScript.
MIT License
27 stars 14 forks source link

Flag more name shadowing - V1 Brighterscript #99

Open markwpearce opened 5 months ago

markwpearce commented 5 months ago

This code was originally in Brighterscript V1, but we elected to not have that as part of the main project, and instead move it to being handled by a linter.

Basically, this would perform a walk on the AST for every file that needs validation in a Scope validation cycle. It finds all namespaces, classes, interfaces, const, enums, etc... that might happen to match a similarly named thing.

Alos, this code was in brighterscript/Scope, so that's what this is refering to here:

    private detectNameCollisions(file: BrsFile) {
        file.ast.walk(createVisitor({
            NamespaceStatement: (nsStmt) => {
                this.validateNameCollision(file, nsStmt, nsStmt.getNameParts()?.[0]);
            },
            ClassStatement: (classStmt) => {
                this.validateNameCollision(file, classStmt, classStmt.tokens.name);
            },
            InterfaceStatement: (ifaceStmt) => {
                this.validateNameCollision(file, ifaceStmt, ifaceStmt.tokens.name);
            },
            ConstStatement: (constStmt) => {
                this.validateNameCollision(file, constStmt, constStmt.tokens.name);
            },
            EnumStatement: (enumStmt) => {
                this.validateNameCollision(file, enumStmt, enumStmt.tokens.name);
            }
        }), {
            walkMode: WalkMode.visitStatements
        });
    }

    validateNameCollision(file: BrsFile, node: AstNode, nameIdentifier: Token) {
        const name = nameIdentifier?.text;
        if (!name || !node) {
            return;
        }
        const nameRange = nameIdentifier.range;

        const containingNamespace = node.findAncestor<NamespaceStatement>(isNamespaceStatement)?.getName(ParseMode.BrighterScript);
        const links = this.getAllFileLinks(name, containingNamespace, true); // the true here is import!
        for (let link of links) {
            if (!link || link.item === node) {
                // refers to same node
                continue;
            }
            if (isNamespaceStatement(link.item) && isNamespaceStatement(node)) {
                // namespace can be declared multiple times
                continue;
            }

            const thisNodeKindName = util.getAstNodeFriendlyName(node);
            const thatNodeKindName = link.file.srcPath === 'global' ? 'Global Function' : util.getAstNodeFriendlyName(link.item) ?? '';

            let thatNameRange = (link.item as any)?.tokens?.name?.range ?? link.item?.range;

            if (isNamespaceStatement(link.item)) {
                thatNameRange = (link.item as NamespaceStatement).getNameParts()?.[0]?.range;
            }

            const relatedInformation = thatNameRange ? [{
                message: `${thatNodeKindName} declared here`,
                location: util.createLocation(
                    URI.file(link.file?.srcPath).toString(),
                    thatNameRange
                )
            }] : undefined;

            this.diagnostics.push({
                file: file,
                ...DiagnosticMessages.nameCollision(thisNodeKindName, thatNodeKindName, name),
                origin: DiagnosticOrigin.Scope,
                range: nameRange,
                relatedInformation: relatedInformation
            });
        }

    }