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

Explore constraint-based analysis #207

Open
muglug opened this issue Sep 3, 2017 · 1 comment

Comments

@muglug
Copy link
Member

commented Sep 3, 2017

The following code has totally valid function type declarations, but those declarations aren't sufficient to detect the erroneous call to foo on the last line.

function takesString(string $s) : void {}

function foo(array $arr) : void {
    foreach ($arr as $a) {
        takesString($a); // MixedArgument issue emitted
    }
}

foo([1, 2, 3]); // No issue emitted

While there's a MixedArgument issue emitted on line 5, if those issues are ignored (as they are by most users of Psalm) we still want an issue to be emitted on line 9.

This can be mitigated by making the param type of $a more explicit on line 3, via a docblock:

function takesString(string $s) : void {}

/** @param string[] $arr **/
function foo(array $arr) : void {
    foreach ($arr as $a) {
        takesString($a);
    }
}

foo([1, 2, 3]); // InvalidArgument issue emitted

However, often people don't bother with such specific docblocks. Flow uses a system of constraints, based upon @fpottier's paper, which Psalm should emulate, possibly via a config (as performance will be affected).

Analysing the function foo (as initially presented) would generate a constraint tree:

$foo_constraints = [
  0 => [
    new TypeConstraint('array'),
    new IteratorConstraint([
        new TypeConstraint('string')
    ]),
  ]
];

Which we would subsequently compute with array<int, int> to find the contradiction.

This could also improve the quality of the types provided in #204

@muglug muglug added the enhancement label Sep 3, 2017

@robehickman

This comment has been minimized.

Copy link

commented Apr 9, 2019

Would a constraint system of this type be able to handle a wordpress style callback system? Within this system, the valid callbacks are defined by all calls to add_action taking a name and a callable. They can be called with 'do_action' passing the same name.

add_action('foo', function(){
   print('here');
});

do_action('foo');

There may be some value in checking that the strings used in 'do_action' calls actually correspond to the ones given in 'add_action' calls. In theory this should just be a matter of getting the first argument of all instances of both functions, then removing the strings from the do_action set that exist in the add_action set. Anything left over being undefined.

Doing it in practise on a wordpress system and it's plug-ins could be difficult, as actions can be undefined with remove_action. Thus, they may not actually exist. Also, some of these names are generated dynamically from content types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.