Code Monkey home page Code Monkey logo

noverify's People

Contributors

abbit avatar blascsi avatar bvwells avatar danil42russia avatar dhtdht020 avatar edgardmessias avatar i582 avatar idevoid avatar lebedevsergey avatar ludweeg avatar mryadro avatar ngkoshkin avatar proggga avatar quasilyte avatar raphaelts3 avatar rukus67 avatar serafimarts avatar sergey-shambir avatar setpill avatar sosiska avatar tamaravedenina avatar tonygoold avatar u5surf avatar vajexal avatar whisk avatar yuriynasretdinov avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

noverify's Issues

False positive triggering for traits

Source Code

<?php
declare(strict_types=1);

trait Example
{
    private static $property = 'some';

    protected function some(): string
    {
        return self::$property;
    }
}

Actual

ERROR   Property does not exist at ...\Example.php:11
        return self::$property;
                     ^^^^^^^^^

Expected

All OK

False negative triggering for IteratorAggregate interface

Code Example

<?php
declare(strict_types=1);

class Example implements \IteratorAggregate
{
   public function getIterator(): array
   {
       return [1, 2, 3];
   }
}

foreach (new Example() as $i) {
   echo $i;
}

Actual Behavior

All OK

Expected Behavior

Error: Objects returned by Example::getIterator() must be traversable or implement interface Iterator

New checks: void type in functions

Code Example

// Warn that "void" is not a valid for input parameter.
/**
 * @param void $x
 * @return void
 */
function f($x) {}

Actual Behavior

No warnings.

Expected Behavior

Warning about void used as a type for "param"/"property" annotation.

Add support of disable/ignore comment to skip error at line

Code Example

for example in pylint we have this comments

import config.logging_settings # pylint: disable=W0611

import config.logging_settings # pylint: disable=unused-import

Sometimes you have strange cases, where linter is not good, but it works
for example old, legacy code

I want some kind of this exclude line by special comment with disable name

$this->gmail->sendEmail($email, $subject);

Here I got:
ERROR   Call to undefined method {}->sendEmail()

I want

$this->gmail->sendEmail($email, subject);  // noverify: disable=call-undefined-method

Add "-output" argument for report files

Description

The console does not have enough buffer (wow!) to display all information, so I think it is worth adding an argument for directory or file for reports.

Suggest

Add -output=./file.txt to output the report to a file.

Example

10

Linter check failed: Unused variable is used

Code Example

$ads_ids = array_keys($arr);
foreach ($ads_ids as $num => $ad_id) {
  if ($num + 1 < count($ads_ids)) {
    $second_ad_id = $ads_ids[$num + 1];
    $arr[$ad_id]->a = $arr[$second_ad_id]->b;
  }
}

Actual Behavior

UNUSED Unused variable ad_id (use $_ to ignore this inspection) at
foreach ($ads_ids as $num => $ad_id) {

Expected Behavior

No errors.

Type inference does not work for variadic functions

class TestClass
{
    public function get(): string
    {
        return '.';
    }
}

function a(TestClass ...$testClasses): string
{
    $result = '';
    foreach ($testClasses as $testClass) {
        $result .= $testClass->get();
    }

    return $result;
}

echo a(new TestClass()), PHP_EOL;
noverify -stubs-dir=~/sources/phpstorm-stubs -cache-dir=~/.cache/noverify -exclude '(vendor|cache)' ~/sources/php-tmp/nover/
2019/03/01 20:23:27.819049 Indexing [~/sources/php-tmp/nover/]
2019/03/01 20:23:27.820374 Linting
ERROR   Call to undefined method {foreach_value}::get() at ~/sources/php-tmp/nover/test.php:22
        $result .= $testClass->get();
                               ^^^
2019/03/01 20:23:27.821266 Found 1 critical reports

Check for duplicate array keys

Code Example

<?php
$config = [ 'key1' => 'a', 'key2' => 'b', 'key1' => 'c' ];

Actual Behavior

All ok.

Expected Behavior

Duplicate key 'key1' in array.

Make all checks named, add enable/disable options

Proposal:

  1. Make all checks named, so they can be addressed.
  2. Add -enable option to turn on checks by a list of permitted checks.
  3. (Optional) Add -disable option to disable a list of checks from the default set.
  4. Update README.

Support `static` and `$this` types

Code Example

class Foo
{
    /** @return $this */
    public function bar()
    {
        return $this;
    }

    /** @return static */
    public function baz()
    {
        return new static();
    }
}

Actual Behavior

For static, NoVerify behaves like it's self, which is not correct (since we need late static binding in those cases instead of an early one).

For $this it infers the "identity" type of $this, which is not useful. Whether it should map it into self or static is an open question though. I would stick to static in this case, but it could be more conservative to use self there, at least for now.

Expected Behavior

static resolved into derived type on the use spots.
$this is either ignored since NoVerify can infer actual return types for return $this or mapped into some other type. It's also possible to handle $this in a special way, but it doesn't seem to yield any benefits over other forms (static/self).

See also yiisoft/yii2#7045.

Failed parsing: Error renaming tmp file

Steps

1)Execute go get -u github.com/VKCOM/noverify
2)Execute

noverify -stubs-dir=D:\\Libs\\phpstorm-stubs -cache-dir=D:\\tmp\\noverify D:\\Dev\\php\\RobotSharpApi -exclude='vendor/'

Actual

Many similar mistakes:

Failed parsing D:\Libs\phpstorm-stubs\zmq\zmq.php: rename D:\tmp\noverify\D_\Libs\phpstorm-stubs\zmq\zmq.php.4687de6d3e2dce931ca5e9470c5b8079.tmp D:\tmp\noverify\D_\Libs\phpstorm-stubs\zmq\zmq.php.4687de6d3e2dce931ca5e9470c5b8079: The process cannot access the file because it is being used by another process.
Failed parsing D:\Dev\php\TestProject\src\Api\ApiLogin.php: rename D:\tmp\noverify\D_\Dev\php\TestProject\src\Api\ApiLogin.php.e3e9f90751d8aba6be7c13558e537d91.tmp D:\tmp\noverify\D_\Dev\php\TestProject\src\Api\ApiLogin.php.e3e9f90751d8aba6be7c13558e537d91: The process cannot access the file because it is being used by another process.

Expected

No error

Constants from interfaces does not recognized

interface TestInterface
{
    const TEST = '1';
}

class TestClass implements TestInterface
{
    public function get()
    {
        return self::TEST;
    }
}
noverify -stubs-dir=~/sources/phpstorm-stubs -cache-dir=$HOME/.cache/noverify ~/php-tmp/nover/
2019/03/01 20:03:50.984902 Started
2019/03/01 20:03:51.429256 Indexing [~/sources/php-tmp/nover/]
2019/03/01 20:03:51.430637 Linting
ERROR   Class constant does not exist at ~/sources/php-tmp/nover/test.php:12
        return self::TEST;
                     ^^^^
2019/03/01 20:03:51.431517 Found 1 critical reports

Port checks from `php -l`

This is an epic-like (progress tracking) issue for porting checks that are implemented in php -l, but are not included in noverify.

The main benefit is to make it possible to remove php -l from the pipeline and use noverify as a drop-in replacement.

Reduce the amount of os.Exit/log.Fatal calls

Ideally, there is only one or two exit points and all other code paths do panic in case of emergency instead of exit.

The problem with exit is that it will prevent all prepared defers to run.
Imagine we executed a bunch of defers inside main, then some of them along the way, during the linter normal work, and we reached exit call. All defers are lost.

Panics can be caught or they can even be left unrecovered, so we get the stack trace of whereabouts of an unexpected condition.

I want to add memprofile and cpuprofile flags, so profiling is more akin to other Go tools, but that is simpler with defers. First step is to remove log.Fatal/os.Exit from the cmd.Main.

Add "-exclude" argument

Description

Each modern project contains a vendor directory. If include the src directory, then the linter produces a lot of false positive errors associated with external dependencies. If add the whole project, the linter will start to produce errors in the dependencies (from the vendor directory).

Suggest

Add "-exclude=./vendor -exclude=./tests -exclude=./resources" (etc) arguments for exclude directories from the report.

Add a project logo

  • Discuss potential options
  • Choose a best fit option
  • Set chosen option as a project logo

Parsing fails with right-to-left text

Code Example

<? echo "اُردُو‎\n"; ?>

Actual Behavior

The linter has a syntax error and is unable to examine the file.

ERROR syntax: Syntax error: syntax error: unexpected $unk at /path/to/utf8.php:2

If the text is inside a class method, it can also show up as an "Empty root node" error.

Expected Behavior

There should be no error.

Notes

This appears to be a php-parser issue that was fixed, so the solution would be to upgrade the vendored php-parser library to the latest version.

False positive triggering of generators

Example

<?php
declare(strict_types=1);

function a($a): \Generator
{
    yield $a;
}

a(42)->send(42);

Actual

ERROR   Call to undefined method {}::send() at ...\Example.php:15
a(42)->send(42);
       ^^^^

Expected

All OK

Wrong interface inheritance handling

Code Example

interface TestInterface
{
    public function getCreatedAt(): \DateTimeInterface;
}

interface TestExInterface extends TestInterface
{
}

function a(TestExInterface $testInterface): string
{
    return $testInterface->getCreatedAt()->format('U');
}

Actual Behavior

2019/03/04 11:37:47.663297 Indexing [~/sources/php-tmp/nover/]
2019/03/04 11:37:47.670811 Linting
ERROR   Call to undefined method {\TestExInterface}->getCreatedAt() at ~/sources/php-tmp/nover/test.php:22
    return $testInterface->getCreatedAt()->format('U');
                           ^^^^^^^^^^^^
ERROR   Call to undefined method {}->format() at ~/sources/php-tmp/nover/test.php:22
    return $testInterface->getCreatedAt()->format('U');
                                           ^^^^^^
2019/03/04 11:37:47.671781 Found 2 critical reports

Expected Behavior

All OK

Steps To Reproduce

noverify -stubs-dir=~/sources/phpstorm-stubs -cache-dir=~/.cache/noverify -exclude '(vendor|cache)' ~/sources/php-tmp/nover/

Environment

  • NoVerify Version: dev-master
  • Operating System: Linux

Anonymous class functions not found

First off this project is looking very promising, I have tried this as a languageserver in vim and it is significantly faster/usable than any other php language servers I've come across 👍

I've found that functions in anonymous classes are not picked up correctly:

Code Example

<?php

$foo = new class() extends \DateTime
{
    public function foo() { }
};

$foo->foo();
$foo->getTimestamp();

Actual Behavior

ERROR   undefined: Call to undefined method {}->foo() at Example.php:8
$foo->foo();
      ^^^
ERROR   undefined: Call to undefined method {}->getTimestamp() at Example.php:9
$foo->getTimestamp();
      ^^^^^^^^^^^^

Expected Behavior

No errors to be reported as foo() and getTimestamp() are valid functions

AST->IR experiment

In a new branch:

  • Implement AST->IR conversion
  • Re-write all analysis from AST to IR
  • Check how it affected performance (run time + allocs)

Goals:

  • Less dependencies on parser-related packages
  • Simpler code inside analysis

Use types info from `__get`, if available

Code Example

<?php

class Magic {
  /** @return Magic */
  public function __get($key) {
    return $this;
  }

  public function method() {}
}

$m = new Magic();
$m->method();
$m->foo->method();
$m->foo->bar->method();

Actual Behavior

Gives several warnings:

ERROR   undefined: Call to undefined method {}->method() at /home/isharipov/CODE/php/hello.php:14
$m->foo->method();
         ^^^^^^
ERROR   undefined: Property {}->bar does not exist at /home/isharipov/CODE/php/hello.php:15
$m->foo->bar->method();
         ^^^
ERROR   undefined: Call to undefined method {}->method() at /home/isharipov/CODE/php/hello.php:15
$m->foo->bar->method();

The problem is that we treat __get as something that returns unknown type. Sometimes that type is known and we can use that to avoid false positives that are given in the case above.

Expected Behavior

No warnings generated.

Note: Phpstorm handles the described case correctly

False positive triggering for closure rebinding

Example

<?php

declare(strict_types=1);

class Example
{
    public function method()
    {
        return 42;
    }
}

(function() {
    $this->method();
})->call(new Example());

Actual

ERROR   Undefined variable: this at ...\Example.php:14
    $this->method();
    ^^^^^
ERROR   Call to undefined method {}::method() at ...\Example.php:14
    $this->method();
           ^^^^^^
ERROR   Call to undefined method {}::call() at ...\Example.php:15
})->call(new Example());
    ^^^^
2019/03/02 10:31:06.650272 Found 3 critical reports

Expected

All OK

Exclude does not exclude

Steps

1)Execute go get -u github.com/VKCOM/noverify
2)Execute

-stubs-dir=D:\Libs\phpstorm-stubs -cache-dir=D:\tmp\noverify -exclude='(vendor|test)' D:\Dev\php\TestProject

Actual

Folders from the exception get into the report

Expected

All OK

Fix

Use -exclude=(vendor|test) or -exclude="(vendor|test)"

Incorrect path definition for cache

Steps To Reproduce

  1. Execute go get -u github.com/VKCOM/noverify
  2. Execute git clone [email protected]:JetBrains/phpstorm-stubs.git D:\\php\\phpstorm-stubs
  3. Execute noverify -stubs-dir=D:\\php\\phpstorm-stubs -cache-dir=./cache ./src

Actual

  1. Multiple errors like: Failed parsing D:\php\phpstorm-stubs\zend\zend_f.php: open .cache\D:\php\phpstorm-stubs\zend\zend_f.php.e3a8c59daf7cb0d1249aa10a581ba9a9.tmp: The filename, directory name, or volume label syntax is incorrect.

Expected

All OK

Suggest

Add support for relative paths

information about instance of variable not used

Code Example

<?php
declare(strict_types=1);

class Element {
    public function get(): int {
        return 0;
    }
}

class Test {
    private $arr = [];

    public function test($key) {
        if (isset($this->arr[$key]) && $this->arr[$key] instanceof Element) {
            return $this->arr[$key]->get();
        }
    }
}

Actual Behavior

ERROR Call to undefined method {}->get()

Expected Behavior

All ok

Environment

  • NoVerify Version: dev-master

Add new type of alerts: useless if or same variable set

Code Example

<?php
define('SOME_CONSTANT', 100500);
$param = $argv[1];
$value = mcGet('some_key');
if (!$data) {
  if ($param) {
    $data = SOME_CONSTANT;
  } else {
    $data = SOME_CONSTANT;
  }
}
echo $data . PHP_EOL;
function mcGet($key) {
  return null;
}

In this case $data has same value in all if cases
I want add check to add alert for unnecessary if or same set variable in all if branches

Improve auto-gen files detection

Currently, an unreliable method is used.

We may also want to accept autogen files patterns instead of recognizing only a small set of possible comment formats.

Invlalid interface methods handling

interface TestClassInterface
{
    public function getCreatedAt(): \DateTimeInterface;
}

function a(TestClassInterface $testClass): string
{
    return $testClass->getCreatedAt()->format('U');
}
noverify -stubs-dir=~/sources/phpstorm-stubs -cache-dir=~/.cache/noverify -exclude '(vendor|cache)' ~/sources/php-tmp/nover/
2019/03/01 20:12:50.227083 Started
2019/03/01 20:12:50.581958 Indexing [~/sources/php-tmp/nover/]
2019/03/01 20:12:50.583191 Linting
ERROR   Call to undefined method {}::format() at ~/sources/php-tmp/nover/test.php:20
    return $testClass->getCreatedAt()->format('U');
                                       ^^^^^^
2019/03/01 20:12:50.584107 Found 1 critical reports

Float-typed values should have "float" type, not "double"

PHP only has 1 floating point type - float.

All of these produce float-typed values:

<?php

var_dump((float)1);
var_dump((double)1);
var_dump((real)1);

Since we do suggest using float inside phpdoc, linter should also infer types as float, otherwise we can't match suggested float with double.

The entire fix is to change "double" strings to "float" inside solver code (I guess that's enough).

Says unreachable code, but it's not

Code Example

<?php

$x = 0;
do {
  test($t);
  $t=1;
  $x++;
}while($x <= 2);

function test($tmp){
  echo "test".$tmp."\n";
}

Actual Behavior

ERROR Undefined variable: t at 1.php:6
test($t);
^^
INFO Unreachable code at 1.php:7
$t=1;
^^^^^

Expected Behavior

Unreachable code ??? not really

Make NoVerify more CI-friendly

Several users reported that NoVerify is too noisy by default and has some false positives that are hard to fix inside linter without introducing false negatives.

The proposal is to add CI-friendly mode that sacrifices some reports for the less false negative rate. More strict (currently the only one) mode can be enabled later when the majority of the project complies to a linter.

Right now I don't know the best way to do it and adding another configuration flag may be sub-optimal. Maybe we need to change default behavior accordingly and only after that consider whether we need very strict mode (needs more feedback and investigation). Discussions are welcome.

Below are some specific complaints, grouped by the check name.

undefined check

  • Don't warn on undefined method/property if the type is unknown. In other words, don't say something like property {}->foo does not exist. For the most strict mode (current) it could be nice to warn that type might not always be what the user expects and that it can't be inferred from the source code, but for CI it's more trouble than it worth.

  • Don't warn on special global variables like $argv in global scope. 90% of their usages are inside scripts that are not included from other scripts, so global statement can be considered too pedantic there. For strict mode it's OK though.

Switch "case" clauses are not examined

Code Example

global $a;
global $b;

switch ($a) {
case $b['key']: // <- this case is not examined
  // ...
}

Actual Behavior

noverify gives "unused b" warning because it doesn't walk case clauses.

Expected Behavior

No warning about unused variable, since it's used inside switch case.

False positive triggering for magic methods

Example

<?php

declare(strict_types=1);

class Magic
{
    public static function __callStatic($name, $arguments): int
    {
        return 42;
    }
}


Magic::some();

Actual

ERROR   Call to undefined method \Magic::some() at ...\Example.php:14
Magic::some();
       ^^^^

Expected

All OK

Failed parsing: If condition containing isset with variable variable expression

When an if condition contains an isset that uses a "variable variable", linter.(*BlockWalker).handleIf panics because it expects a *node.Identifier for the variable name instead of a *expr.Variable.

Code Example

function get_global_var($name) {
    global $$name;
    if (isset($$name)) { // Panic occurs while parsing this line
        return $$name;
    }
    return null;
}

Actual Behavior

Panic while parsing <filename>: interface conversion: node.Node is *expr.Variable, not *node.Identifier

Expected Behavior

No panic.

Add support of deprecated functions.

Code Example

mysql('a', 'b', 'c');

Actual Behavior

All OK

Expected Behavior

Notice: mysql function is deprecated since 5.3.0

Suggest

You can analyze @deprecated annotations from phpstorm-stubs (and not only). In addition, @internal annotations can be read.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.