Page MenuHomePhabricator

Continuous Integration should offer a LESS linter
Closed, ResolvedPublic

Description

Manual:Coding_conventions/CSS proposes some standards for CSS and LESS files. Projects should be able to opt in to the enforcement of these standards by adding a CSS linter to their build checks, similar to how they run jshint and phpcs for JavaScript and PHP coding conventions. Several projects have a csslint rule in Gruntfile.json, but csslint isn't aware of LESS syntax.

Possibilities:

  • There is a LESS Lint Grunt plugin that works by compiling LESS files into CSS using the npm less and running csslint on its output. But that output may not match the CSS generated by the PHP LESS compiler (leafo/lessphp) that MediaWiki's ResourceLoader uses.
  • recess is a native LESS linter.

Details

Related Gerrit Patches:

Event Timeline

Spage raised the priority of this task from to Needs Triage.
Spage updated the task description. (Show Details)
Spage added subscribers: Spage, Volker_E, Krinkle.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 24 2015, 2:46 AM

When the MediaWiki structure tests run for extensions (not skins yet), all less files that are part of a resourceloader module have their less files compiled to make sure they do in fact compile.

FWIW after npm install recess

$ ./node_modules/.bin/recess  --includePath=resources/src/mediawiki.less resources/src/mediawiki.ui/components/utilities.less

Analyzing the following files: resources/src/mediawiki.ui/components/utilities.less

FILE: resources/src/mediawiki.ui/components/utilities.less
STATUS: Busted
FAILURES: 2 failures

Incorrect property order for rule ".mw-ui-flush-left"

  Correct order below:

       2.  float: left;
       3.  padding-left: 0;
       4.  margin-left: 0;

Incorrect property order for rule ".mw-ui-center-block"

  Correct order below:

      12.  display: block;
      13.  margin-right: auto;
      14.  margin-left: auto;

but it complains

undefined: 'mediawiki.mixins' wasn't found.

I think recess like other less compilers requires @imported files to include the extension, e.g. @import "mediawiki.mixins.less";

hashar added a subscriber: hashar.

That is built-in MediaWiki core:

tests/phpunit/LessFileCompilationTest.php
tests/phpunit/suites/LessTestSuite.php

That is part of the PHPUnit 'extensions' suite (tests/phpunit/suite.xml):

<testsuite name="extensions">
    <directory>structure</directory>
    <file>suites/ExtensionsTestSuite.php</file>
    <file>suites/ExtensionsParserTestSuite.php</file>
    <file>suites/LessTestSuite.php</file>  <----------------- there
</testsuite>

Apparently the suite tests/phpunit/LessFileCompilationTest.php is not included when executing php phpunit.php :-/

You can try sending a change against mediawiki/core that introduces a bad .less and see what happens :)

Change 227232 had a related patch set uploaded (by Hashar):
Actually run the Less compilation tests

https://gerrit.wikimedia.org/r/227232

The less compilation tests are not taken in account by PHPUnit. https://gerrit.wikimedia.org/r/227232 should fix it. We will want to introduce the fix to the supported release branches as well.

The test does not show up in the test report:

https://integration.wikimedia.org/ci/job/mediawiki-phpunit-hhvm/11856/testReport/(root)/

I am not sure it is actually taken in account.

Change 227237 had a related patch set uploaded (by Hashar):
Test Less failure in mediawiki.debug.less

https://gerrit.wikimedia.org/r/227237

hashar added a comment.EditedJul 27 2015, 4:45 PM

I sent a faulty change https://gerrit.wikimedia.org/r/#/c/227237/ as a follow up. The job fails as expected:

00:03:14.947 1) resources/src/mediawiki/mediawiki.debug.less in the "mediawiki.debug" module
00:03:14.947 Exception: parse error: failed at `.mw-debug` resources/src/mediawiki/mediawiki.debug.less on line 1

The Jenkins test report not capturing that failure is probably unrelated.

We surely want to refactor this test to use a PHPUnit data provider which will give a much cleaner output.

hashar triaged this task as High priority.Jul 27 2015, 4:47 PM

That is probably a high priority item though I have no spare time left to work on it :-(

Change 227232 merged by jenkins-bot:
Actually run the Less compilation tests

https://gerrit.wikimedia.org/r/227232

Change 227237 abandoned by Hashar:
Test Less failure in mediawiki.debug.less

https://gerrit.wikimedia.org/r/227237

That is built-in MediaWiki core ... LessFileCompilationTest.php

AIUI that tests for errors, not coding conventions as a LESS linter would. Same difference as php -l and phpcs. The error code I listed resulted from a quick hack trying out the recess linter, it's not what I'm trying to check.

Is there anything left to do here? AFAIS all MW extensions running the normal tests will have their less files linted.

Legoktm closed this task as Resolved.Oct 27 2015, 11:04 PM
Legoktm claimed this task.
Legoktm reassigned this task from Legoktm to hashar.

As both linters in task description are not longer maintained, https://github.com/lesshint/lesshint might be an useful alternative.

hashar added a comment.Mar 9 2016, 8:14 PM

We have phpunit based test:

tests/phpunit/LessFileCompilationTest.php
tests/phpunit/suites/LessTestSuite.php

It invokes the MediaWiki resource loader method compileLessFile which itself should use the library "oyejorge/less.php": "1.7.0.10", (from composer.json).

That does not cover style consistency though.

As additional accomplishment note, stylelint fullfills all needs of modern pre-processor (Less, Sass) and CSS linting needs.