The webprofiler sub-module of Devel has the following [blank lines removed]

namespace Drupal\webprofiler {
  /**
   * Class Stopwatch.
   */
  class Stopwatch extends \Symfony\Component\Stopwatch\Stopwatch {    <<< this is line 8
  }
}

Coder reports

8 | ERROR   | [x] Namespaced classes/interfaces/traits should be referenced with use statements
(Drupal.Classes.FullyQualifiedNamespace.UseStatementMissing)

The automatcic fix produces the patch:

+use Symfony\Component\Stopwatch\Stopwatch;
+
 namespace Drupal\webprofiler {
   /**
    * Class Stopwatch.
    */
-  class Stopwatch extends \Symfony\Component\Stopwatch\Stopwatch {
+  class Stopwatch extends Stopwatch {
   }

which results in:

use Symfony\Component\Stopwatch\Stopwatch;
namespace Drupal\webprofiler {
  /**
   * Class Stopwatch.
   */
  class Stopwatch extends Stopwatch {   <<< this is line 10
  }
}

and this fails with

PHP Fatal error:  Class 'Drupal\\webprofiler\\Stopwatch' not found in /devel/webprofiler/src/Stopwatch.php on line 10

Maybe in cases like this, where the class name matches the extending name PHPCS could report a warning, but not add the auto-fixer?

CommentFileSizeAuthor
#4 coder-3137489-4.patch1.28 KBcburschka

Comments

jonathan1055 created an issue. See original summary.

jonathan1055’s picture

I have fixed it manually:

 namespace Drupal\webprofiler {
 use Symfony\Component\Stopwatch\Stopwatch as SymfonyStopwatch;
  /**
   * Class Stopwatch.
   */
  class Stopwatch extends SymfonyStopwatch { 
   }
}

But I don't think this should be attempted automatically as it involves making a decision on what alias name to use. So it should just give a warning but no fixer. I will try to find how to alter the sniff to avoid this.

arkener’s picture

Good catch!, we recently fixed this issue for conflicts between use-statements in https://www.drupal.org/node/3112316, but we never took the currently class into consideration. This means that this issue also occurs if the current class is used as a parameter type, for example:

namespace Drupal\project;

use Drupal\otherproject\Bar;

class Foo extends Bar {
  public function lorem(\Drupal\project\Foo $var) {}
}

Results in the following:

namespace Drupal\project;

use Drupal\project\Foo;
use Drupal\otherproject\Bar;

class Foo extends Bar {
  public function lorem(Foo $var) {}
}

A good first step would be to just skip the auto-fix if the class names might conflict.

cburschka’s picture

Status: Active » Needs review
StatusFileSize
new1.28 KB

Here's a first attempt at expanding the conflict detection to include named classes declared in the file.

arkener’s picture

Status: Needs review » Needs work

Thanks for the patch, can you create a pull request against https://github.com/pfrenssen/coder so that we can check the test cases? We should probably also add a test case for this.

cburschka’s picture

Status: Needs work » Needs review

Sure! https://github.com/pfrenssen/coder/pull/109

(I've also fixed a variable collision issue in my code that overwrote $after, so the PR is now newer than the patch here.)

arkener’s picture

Status: Needs review » Needs work

Thanks! I've left a comment with a request for some minor changes on the PR.

  • cburschka authored f79cef4 on 8.x-3.x
    feat(FullyQualifiedNamespace): Disable fixer on conflict between FQN...
arkener’s picture

Status: Needs work » Fixed

Thanks a lot, merged!

jonathan1055’s picture

Been following this on the MR. Just downloaded latest 3.x dev and confirm, using the original example I reported, that the output is now

8 | ERROR   | [ ] Namespaced classes/interfaces/traits should be referenced with use statements (Drupal.Classes.FullyQualifiedNamespace.UseStatementMissing)

without the automatic fixer flag.

Thanks @cburschka and @Arkener.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.