Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

no-unbound-method in Jasmine spy have been called test #3755

Closed
piotrgajow opened this issue Mar 5, 2018 · 12 comments
Closed

no-unbound-method in Jasmine spy have been called test #3755

piotrgajow opened this issue Mar 5, 2018 · 12 comments

Comments

@piotrgajow
Copy link
Contributor

Bug Report

  • TSLint version: 5.9.1
  • TypeScript version: 2.5.3
  • Running TSLint via: Node.js API

TypeScript code being linted

it('on destroy should unsubscribe timer', () => {
    component.timerSubscription = jasmine.createSpyObj('Subscription', ['unsubscribe']);

    component.ngOnDestroy();

    expect(component.timerSubscription.unsubscribe).toHaveBeenCalled();
});

with tslint.json configuration:

{
    "rulesDirectory": [
        "node_modules/codelyzer"
    ],
    "rules": {
        "no-unbound-method": true
    }
}

Actual behavior

tslint reports error in the expect line on component.timerSubscription.unsubscribe - Avoid referencing unbound methods which may cause unintentional scoping of 'this'.

Expected behavior

This only references the method to check if it has been called - it does not call it, nor it saves it into variable for later calls - so it should not report any errors.

@grimly
Copy link

grimly commented Apr 4, 2018

The same happen on type detection.

This code throws a no-unbound-method error

const property = Object.getOwnPropertyDescriptor(target, propertyName);
if (typeof property === 'object' && typeof property.set === 'function') {
  // ...
}

@maxi7587
Copy link

maxi7587 commented May 3, 2018

Have the same issue, how do you use expect() actually with tslint 5.9.1 in jasmine?

@Plondrein
Copy link

any update on this?

@chris-putnam
Copy link

same issue - same exact test code as the OP actually

@robvaneck
Copy link

I have the same issues since I updated packages.json

    "@angular-devkit/build-angular": "^0.7.5",
    "@angular/cli": "^6.2.8",
    "@angular/compiler-cli": "^6.1.10",
    "@angular/language-service": "^6.1.10",
    "@types/jasmine": "^2.8.14",
    "@types/jasminewd2": "^2.0.6",
    "@types/node": "~8.9.4",
    "codelyzer": "^4.5.0",
    "jasmine-core": "~2.99.1",
    "jasmine-spec-reporter": "~4.2.1",
    "karma": "~1.7.1",
    "karma-chrome-launcher": "~2.2.0",
    "karma-coverage-istanbul-reporter": "^2.0.4",
    "karma-jasmine": "~1.1.1",
    "karma-jasmine-html-reporter": "^0.2.2",
    "protractor": "^5.4.1",
    "ts-node": "~5.0.1",
    "tslint": "^5.12.0",
    "typescript": "^2.9.2"

@JoshuaKGoldberg
Copy link
Contributor

Just to make what's expected here clear: the no-unbound-method rule doesn't have a configuration setting to allow a whitelist of methods to call with. For example, "expect" should be allowed as a whitelisted method to stop complaining about in the OP. Still accepting PRs to add that in.

@piotrgajow
Copy link
Contributor Author

I will try to work on this issue and prepare a PR, but this will be my first contribution here, so it might take some time :). If you have any tips on where to start, please let me know.

@JoshuaKGoldberg
Copy link
Contributor

Awesome, thanks @piotrgajow! https://palantir.github.io/tslint/develop/custom-rules/walker-design.html is a good starting place. https://astexplorer.net with language set to TypeScript is really useful to explore what the AST looks like for code snippets.

Feel free to ask for help on Gitter or Twitter if you have any questions! The docs are a little old and you definitely wouldn't be the only one to want to ask for clarifications!

@piotrgajow
Copy link
Contributor Author

I am wondering how to pass the method/operator names that should be excluded in rule checking. Should the options be changed into an object (e.g. { "ignore-static": true, "whitelist": ["expect", "typeof"] })?

@JoshuaKGoldberg
Copy link
Contributor

Great question... we can't break backwards compatibility for the rule, so it should always allow something like "no-unbound-method": [true, "ignore-static"]. How about allowing either a string or the object as the parameter? Does that look doable with the metadata format?

@piotrgajow
Copy link
Contributor Author

I have made a PR for this issue, please take a look in some spare time and let me know what do you think about it 😄.

@JoshuaKGoldberg
Copy link
Contributor

Fixed by #4472! 🙌

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants