Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

New Rule: All paths in functions should resolve or reject a JQuery deferred #26

Closed
HamletDRC opened this issue Sep 29, 2015 · 3 comments
Milestone

Comments

@HamletDRC
Copy link
Member

New Rule: All paths in functions should resolve or reject a JQuery deferred

This code is OK:

function myMethod() : JQueryPromise<void> {
    var deferred: JQueryDeferred<void> = jQuery.Deferred<void>();
    if (something) {
        deferred.resolve();
    } else {
        if (somethingElse) {
            deferred.resolve();
        } else {
            deferred.reject();
        }
    }
    return deferred.promise();
}

This code triggers a violation because not all paths resolve or reject the deferred:

function myMethod() : JQueryPromise<void> {
    var deferred: JQueryDeferred<void> = jQuery.Deferred<void>();
    if (something) {
        deferred.resolve();
    } else {
        if (somethingElse) {
            deferred.resolve();
        } 
    }
    return deferred.promise();
}

Limitations

Detecting Deferred instantiation - It is not possible to know conclusively when a new instance of JQuery deferred is created. For the purposes of this rule, we consider "$.Deferred" and "jquery.Deferred" to create a new Deferred instance. The "jquery" token will be case-insensitive (so jquery.Deferred and JQUERY.Deferred will both be assumed to be returning new instances). A deferred passed as a parameter or returned as the result of some other function will not be considered a new instance.

Deferred reference escaping - If the deferred object escapes the method (i.e. it is passed to some other function) then we assume it is resolved/rejected on that code path.

Nested Scopes - If the Deferred object is referenced in a nested scope, then we will assume that the nested scope is always invoked. Consider this code:
function myMethod() : JQueryPromise {
var deferred: JQueryDeferred = jQuery.Deferred();

    invokeSomething(() : void => {
        deferred.resolve();
    });
    return deferred.promise();
}

Does the function passed to "invokeSomething" get invoked always or only some time? It is possible in this scenario that invokeSomething does not call the callback, and we then have a path through the function that does not invoke resolve/reject. Consider this code as well:

function myMethod() : JQueryPromise<void> {
    var deferred: JQueryDeferred<void> = jQuery.Deferred<void>();

    if (someArray.length > 0) {
        _.each(someArray, () : void => {
            deferred.resolve();
        });
    } else {
        deferred.reject();
    }
    return deferred.promise();
}

To someone who "knows" underscore, it looks like resolve/reject will always invoked. However, there is no way for us to know that _ represents what is in underscore.d.ts so we cannot assume the _ points to the underscore.js library.

@egamma
Copy link
Member

egamma commented Sep 29, 2015

I would be nice to have a similar rule for ES6 promises as well.

@HamletDRC
Copy link
Member Author

@egamma Do you have an opinion about what should happen inside of a while-loop, a for-loop, or a for-in loop?

Do we assume that the loop will always execute at least once? Does this create a violation?

  var deferred: JQueryDeferred<void> = jQuery.Deferred<void>();
  while(something) {
      deferred.resolve();  
  }
  return deferred.promise();

What about a branch that appears in one of these loops. Do we assume that the while-loop always iterates and there there is always at least one iteration there somethingElse is true?

var deferred: JQueryDeferred<void> = jQuery.Deferred<void>();
while(something) {
    if(somethingElse) {
        deferred.resolve();  
    }
}
return deferred.promise();

@HamletDRC
Copy link
Member Author

This rule is implemented in 0.0.5 with the described limitations.

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

No branches or pull requests

2 participants