This array(1,2,3) passes, but it shouldn't.

This command should do the following

• Provide code coverage for all classes/methods in the namespace
• Only print those out for a given threshold (for example 0 for those with no coverage or 80 for those with a percentage of 80% or less).

Currently, testing does not work on arbitrary paths, and also does not work on procedural PHP files. It should be possible to run tests on any collection of PHP files.

http://dev.lithify.me/lithium/wiki/standards/documenting

There is a section on the docs for "Order of Tags" but it only orders 4 of them "link", "see", "param", and "return".

After reading through the irc logs linked to from the docs, I notice they split the tags into two categories "code specific" and "doc specific". The doc specific should go first and the code specific second (closer to code). Both sets of tags should go in alpha order.

Just for clarification, the "basic" list is the list of code specific tags, and the other are doc specific?

Would we allow other tags (since, final, ignore, ...)? Or is this the complete list of allowed tags?

Ran into a few issues when running my current dev branch on a plugin.

1. Saying "->" isn't spaced correctly.

$blames = Blame::find('all', array(
    'conditions' => array(
        'file' => $file,
        'lines' => array(1,2),
    ),
    'groupBy' => 'name',
))->to('array');

2. Saying this has an inline comment

    /**
     * Part of the Iterator interface
     *
     * @link http://www.php.net/manual/en/class.iterator.php
     * @return mixed
     */

3. Saying incorrect class declaration

class FileRecord extends Record implements Iterator, ArrayAccess {

I'll try and fix these tonight, but needed to write them down so I can remember them.

The Quality command should check the current app to see if the name is app, and if so, show an error saying something like "Your application still uses the default project name and namespace. Please give it a unique name."

It would be great to integrate the syntax checks into the scrutinizer CI. The following document details how this can be achieved:

https://scrutinizer-ci.com/docs/tools/custom-commands/

The syntax command will then need to be able to work with single files (instead of whole libraries). See #10. In general the command should not use the Libraries class to find its files.

Why does the syntax checker for this plugin not follow PSR? I know a lot of code will probably change inside this plugin and lithium itself to follow the PSR rules but surely it makes more sense to follow them than not to? My biggest annoyance of all is the fact that this plugin enforces tabs for indenting and not spaces which goes completely against PSR-2. Can we change this?

I just run li3 quality syntax and found few things concerning PHP 5.4.

• HasCorrectTabIndention break tab counts when PHP5.4 brackets array syntax is used
• Using Traits cause a File does not end with ?> violation and Trailing whitespace found

( ! ) Fatal error: Uncaught exception 'lithium\core\ConfigException' with message 'Configuration li3_quality has not been defined.' in C:\wamp\www\libraries\lithium\core\Adaptable.php on line 112
( ! ) lithium\core\ConfigException: Configuration li3_quality has not been defined. in C:\wamp\www\libraries\lithium\core\Adaptable.php on line 112
Call Stack

Time Memory Function Location

1 0.0008 142952 {main}( ) ..\index.php:0
2 0.2346 2215168 lithium\action\Dispatcher::run( ) ..\index.php:41
3 0.2346 2216240 lithium\core\StaticObject::_filter( ) ..\Dispatcher.php:155
4 0.2346 2217936 lithium\util\collection\Filters::run( ) ..\StaticObject.php:126
5 0.2347 2219968 lithium\core\Libraries::{closure:C:\wamp\www\libraries\li3_docs\config\bootstrap.php:26-38}( ) ..\Filters.php:183
6 0.2349 2220136 lithium\util\collection\Filters->next( ) ..\bootstrap.php:32
7 0.2349 2220224 {closure:C:\wamp\www\app\config\bootstrap\action.php:41-52}( ) ..\Filters.php:202
8 0.2501 2299504 lithium\util\collection\Filters->next( ) ..\action.php:51
9 0.2501 2299536 {closure:C:\wamp\www\app\config\bootstrap\cache.php:41-57}( ) ..\Filters.php:202
10 0.2501 2299624 lithium\util\collection\Filters->next( ) ..\cache.php:43
11 0.2501 2299656 lithium\action\Dispatcher::lithium\action{closure}( ) ..\Filters.php:202
12 0.2554 2337200 lithium\core\StaticObject::invokeMethod( ) ..\Dispatcher.php:154
13 0.2555 2337232 lithium\action\Dispatcher::_call( ) ..\StaticObject.php:75
14 0.2555 2338312 lithium\core\StaticObject::_filter( ) ..\Dispatcher.php:265
15 0.2555 2338528 lithium\action\Dispatcher::lithium\action{closure}( ) ..\StaticObject.php:119
16 0.2555 2338544 lithium\test\Controller->__invoke( ) ..\Dispatcher.php:262
17 0.2555 2341264 lithium\core\Object->_filter( ) ..\Controller.php:59
18 0.2555 2341552 lithium\test\Controller->lithium\test{closure}( ) ..\Object.php:238
19 0.2569 2357792 lithium\test\Dispatcher::run( ) ..\Controller.php:49
20 0.2674 2642552 lithium\core\StaticObject::_filter( ) ..\Dispatcher.php:73
21 0.2674 2642840 lithium\test\Dispatcher::lithium\test{closure}( ) ..\StaticObject.php:119
22 0.2674 2642928 lithium\test\Report->run( ) ..\Dispatcher.php:69
23 0.2712 2658912 li3_quality\extensions\test\filter\Syntax::apply( ) ..\Report.php:142
24 0.3206 2878024 li3_quality\test\Rules::apply( ) ..\Syntax.php:26
25 0.3207 2891808 li3_quality\test\rules\DoesntExceedMaxLineLength->apply( ) ..\Rules.php:48
26 0.3207 2892288 lithium\g11n\Multibyte::strlen( ) ..\DoesntExceedMaxLineLength.php:22
27 0.3207 2892712 lithium\core\Adaptable::adapter( ) ..\Multibyte.php:97

If this is the case, could the composer.json be update so it does not require 1.0.0-RC

I'm trying to run li3_quality against li3_quality and I can't seem to get it to work.

[vagrant@app]$ ../libraries/lithium/console/li3 quality syntax li3_quality
Could not find any files in li3_quality\.

[vagrant@app]$ ../libraries/lithium/console/li3 quality syntax libraries/li3_quality
Could not find any files in li3_quality\.

[vagrant@app]$ ../libraries/lithium/console/li3 quality syntax libraries/li3_quality/tests
--------------------
Lithium Syntax Check
--------------------
Performing 18 rules on 2 classes.
[OK] li3_quality\tests\mocks\test\Testable
[OK] li3_quality\tests\cases\test\rules\ControlStructuresHaveCorrectSpacingTest

Shouldn't the top two work?

Running without the path runs against all files (lithium, app, libraries)

Hi guys,

I'm just started playing with li3_quality and trying the following command:

li3 quality syntax libraries/lithium/data/source/Database.php

And it takes approx. 2min for displaying a result.

Is it the normal behavior ?

--ignore could take a directory or a pattern for ignoring some files.
Example: Ignoring vendordirectory added for 3-rd party libs installed by Composer.

This enhancement should provide the following:

• include/exclude distinct rules to be applied on testables.
• provide your own ruleset that will be applied.

A default template for the Lithium ruleset will be used by default and things can be changed on the fly or overriden from within your application context.

Feel free to your thoughts on this!

Most of our exceptions have a protected $code.

When extending exceptions you have a protected $code however the name goes against our current coding standards.

We could make them public, since in PHP we can always make the visibility of members and methods more visible. This opens up our exceptions to be 'leaky' and edited at runtime.

Or we could make an exception, but this is the tricky part. We could do it for $code with obvious repercussions, or possibly check for "/Exception$/" in class name, but this doesn't ensure that it extends exception. We could open up the file and use Reflection or in_a to determine if it's an exception, but then our static code analyzer becomes a hybrid static/dynamic code analyzer.

Thoughts?

I tried simplifying the rule regarding declaring public/private/protected on methods/variables with the new 'tokenIn' and its giving me unexpected results. I feel I should abstract this, and even move it around. I feel I should simply make a new "parser"class which extends the original and give more details like a t_function could have a parent which would be the id of the t_class it's in. Then things like "tokenIn" would be simpler. I also feel things like "tokenIn" and "findNext" should be in testable instead of Rule. I figure after I get these new methods in place and tested I can rewrite and simplify a few of the current rules.

The current untested implementation has "parent" which is just the id of it's parent (or -1), a children array which has the id of all of it's children, level which is the depth of the token, and some tokens which support names (var, func, class) also have name fields. I figure this would make testing a ton easier with complex searching.

Thoughts?

Currently, the rules are not tested, so it's hard to verify their behaviour.

Testing will be added so that it is immediately clear on what code it will apply or not.

The dev branch looks dead right now. Are there any objections to doing a force push on it then only accepting pull requests on that branch? I think we could start versioning our pre 1.0 plugin.

Should config libraries be in CamelCase? Or should they rule ignore config files? Or ignores files that don't contain classes?

https://github.com/UnionOfRAD/framework/tree/master/app/config/bootstrap

Hi!

Cloned li3_queality to test it out and I'm getting multiple PHP warnings (I have MANY failed checks). The errors:

PHP Strict standards: Only variables should be assigned by reference in /Volumes/Vault/Web/li3/libraries/li3_quality/test/rules/BeginsWithOpenTag.php on line 15

PHP Strict standards: Only variables should be assigned by reference in /Volumes/Vault/Web/li3/libraries/li3_quality/test/rules/EndsWithCloseTag.php on line 15

PHP Strict standards: Only variables should be assigned by reference in /Volumes/Vault/Web/li3/libraries/li3_quality/test/rules/HasNoEOLatEOF.php on line 15

PHP Strict standards: Only variables should be assigned by reference in /Volumes/Vault/Web/li3/libraries/li3_quality/test/rules/HasTabsAsIndentation.php on line 15

PHP Warning: array_slice() expects parameter 1 to be array, object given in /Volumes/Vault/Web/li3/libraries/li3_quality/test/rules/ProtectedMethodStartsWithUnderscore.php on line 19

PHP Warning: Invalid argument supplied for foreach() in /Volumes/Vault/Web/li3/libraries/li3_quality/test/rules/ProtectedMethodStartsWithUnderscore.php on line 34

do..while loops result in a false positive:

Unexpected T_WHILE format. Should be: "while (...) {" or "} while (...); on a signle line.

Operator . must have 1 leading and an optional trailing space.

It's very possible now create configurable rules that are not used by default because of the ruleset I implemented.

However I was considering some new rules I think would be useful.

• Currently we don't have rules on line spacing, however I think 2+ blank lines should throw violations
• Unused use statements, I often find myself and otherdevs leaving these in without realizing
• One @nateabele already mentioned should be in here is having spaces after ";" in for statements, however because it's not written down I keep forgetting to implement.

Thoughts?

I get 'A parse error has been encountered' exception when syntax checking my class. It's a subclass of lithium\template\View, which uses $this->{'_' . $key} = assignments in its _init() method that is causing this problem. I had 2 instance of this assignment and the error logged [queue] => 0 and [level] => -2 with all the source tree (running the syntax command for the li3-core class also yields the error).

This is a collection of rules that need to be implemented in order to fully enforce Lithium's coding standard.

• A single space should always appear between control structure keywords and parentheses, i.e. if (...), switch (...), etc.
• A single space should always appear between any closing parentheses and any opening brace, i.e. ) {
• if clauses and opening braces should always appear on a single line, i.e. if ($condition || $condition) {
• Expressions in control structures should never contain spaces within the parentheses, i.e. if ( isset($config['port']) ) {
• case clauses should always appear on a single line
• case and default clauses should always be indented one level deeper than their corresponding switch statement
• Statements within control structures should always be indented one level deeper than their deeper than their wrapping structures
• Tabs should never appear within lines, only at the beginning of the line
• Docblocks should be indented properly, see UnionOfRAD/li3_qa#1
• Echo statements should not contain any spaces after the opening, and should end with one semicolon and one space before the closing tag, i.e. <?=$post->title; ?>
• All structures including arrays should be indented symmetrically, and the closing parentheses (or brace) for a multi-line array should always be indented at the same level as the array opening.
• All operators should be prefixed and suffixed with exactly one space, i.e.: $x = $y, 'x' . 'y', $x || $y, $x ?: $y, $x ? $x : $y, $x += $y, $x - $y, $x - 23, function($x, $y = null) {}
• Defines and class constants should always be capitalized
• Casts should use long names, and only contain exactly one space between the cast clause and the expression, i.e. (boolean) $foo, (integer) $bar
• Properties and methods should always be declared with visibility
• Language constructs such as include and require should never use parentheses

I implemented the git pre-commit hook (as in the README.md) but using this while committing lithium view files in raw HTML gave me unexpected errors such as:

File does not begin with

Also When a line ends with a period the following line will error with:

Incorrect tab indention 0 should be 1.

I think it makes sense to ship builtin rules for both tab and whitespace indentation.
Too many projects will be using different rules here so it doesn't make sense that every project needs to write custom code just to enforce their standard.

I suggest we rename the HasTabsAsIndentation to HasCorrectIndentation and passes the actual rule as a configuration:

{
    "variables": {
        "HasCorrectIndentation": {
            "character": "\t",
            "num": 1
        }
    }
}

I'll create a PR if theres no disagreement to this.

#90
CI test ran against PHP 5.3 and 5.4 instead of >=5.5 as indicated by the composer.json

use app\models\site\Sections as SiteSections;

If there is more than one whitespace after the as, the additional whitespace is considered to be inside the class name and it will look up that class name for the UnusedUseStatements rule.

(of course, the issue is the fact that the double whitespace shouldn't be there in the first place;) )

I have this in my ~/.vimrc:

set        foldmethod           =marker                   " Enable folding by fold markers

That enables code folding. I generally mark my code as 'foldable' like this:

// {{{ validation
    public $validates = [
    ];
// }}}

However, that yields the following violation:

105     -           Inline comments should never appear.                        

How do you guys have your folding configured so that it doesn't trip up li3_quality?

My file:

<?php

namespace app\models;

/**
 *
 */
class AbActions extends \sc_common\models\AbActions {}

?>

It doesn't understand that I close the class immediatly on the same line, is this a serious issue?

What about to be less strict on line length. Im ok with the principle to keep small line length to improve readability. Narrow code can be read quickly without having to scan your eyes from side to side. And I think everyone try to go that way.

But sometimes the "refactoring" for exceeded with a couple of characters make the result less readabe or "unusual" than the longer version (e.g like defining a function verticaly instead of horizontally). And sometimes renaming a variable to be shorter than the previous one to fit the max len make it less meaningfull. That it is not worth it imo.

Since the core often exceed the 80 char limit, why not using 100 as soft limit. Displaying a warning over 100 (exceeding with just a couple of characters still ok). And having a hard limit to 110 or 120 ?

Or allowing any length for function definition and removing warning for length between 80-100 ?

In the first step, a composer file will be created so that it can be easily used from within a composer project.

This command should

• provide a list of unfinished or undocumented classes/methods
• provide statistics for the above mentioned metrics

Some protected methods from core PHP class can only be overridden or reimplemented without using a leading underscore. This must be an exception.

        if (`lsof "{$file}"`) {
            $this->notice("`{$file}` is currently in use.");
            return false;
        }

This causes a parser error

What exactly is this class testing for?

We already split by \r or \n so finding a \r on a single line is already impossible?

Should it instead be scanning the entire source for a \r? Or is this obsolete now?

I'm trying to get li3_quality to work in my browser's test suite. When I click on Syntax, I get a couple of errors:

Warning: include(): Filename cannot be empty in /var/www/libraries/lithium/test/Report.php on line 236
Failed opening '' for inclusion (include_path='.:/usr/share/php:/usr/share/pear') in /var/www/libraries/lithium/test/Report.php on line 236 

First, I thought it would be OS related, but I get the same errors on both Windows (Zend Server) and Linux (Apache 2).
I'm using the (latest) master branch.