Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No error for missing method call #35557

Closed
mjbvz opened this issue Dec 6, 2019 · 4 comments · Fixed by #35862
Closed

No error for missing method call #35557

mjbvz opened this issue Dec 6, 2019 · 4 comments · Fixed by #35862
Labels
Bug A bug in TypeScript
Milestone

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Dec 6, 2019

TypeScript Version: 3.8.0-dev.20191206

Search Terms:

  • diagnostics
  • conditional
  • missing function call

Code

import * as fs from 'fs';

fs.stat('/path/to/file', function(err, stats) {
    if (stats.isDirectory) { 
        console.log(`[Directory] ${stats.ctime}`)
    }
});

Expected behavior:
Error on isDirectory. This is a method but it has not been called

Actual behavior:
No error reported.

If I remove the console log line

@mjbvz mjbvz changed the title No error for missing method call if other property is accessed in conditional body No error for missing method call Dec 6, 2019
@MartinJohns
Copy link
Contributor

This will only report an error if strictNullChecks is enabled, and stats.isDirectory is a non-nullable function. See #33178 and #9041.

@mjbvz
Copy link
Contributor Author

mjbvz commented Dec 7, 2019

This is in strict mode:

Dec-07-2019 09-36-01

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Dec 9, 2019
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.8.1 milestone Dec 9, 2019
@RyanCavanaugh
Copy link
Member

It looks like we're confused by any reference to stats in the body if the if

@dragomirtitian
Copy link
Contributor

dragomirtitian commented Dec 26, 2019

The way the check is done for usage seems brittle. The only check that is done is for symbol id equality:

const functionIsUsedInBody = forEachChild(ifStatement.thenStatement, function check(childNode): boolean | undefined {
    if (isIdentifier(childNode)) {
        const childSymbol = getSymbolAtLocation(childNode);
        if (childSymbol && childSymbol.id === testedFunctionSymbol.id) { // HERE 
            return true;
        }
    }

    return forEachChild(childNode, check);
});

This has the drawback of not actually working at all for symbols which have not had an id assigned (and we get undefined === undefined). This could be fixed by using getSymbolId which would assign the id, or by direct object equality (since as far as I understand it the same symbol object should be used and id equality when assigned at this location seems no better) but this fix would not fix the following surprising behavior:

interface StatsBase {
    isDirectory(): boolean;
}
function A(stats2: StatsBase, stats: StatsBase) {
    if (stats.isDirectory) { // no error because we access isDirectory on stat2
        stats2.isDirectory(); // comment this and we get an error above
    }
}

Playground Link

The symbols isDirectory is used, in the if condition and in the body but the target is different.

Proposed solution:

  1. Use getSymbolId for when comparing symbol ids
  2. Compare the expression of the property access to ensure it is the same (if the testedNode is a property access)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants