Problem/Motivation

Discovered in #3260401: Google is abandoning FLoC - so remove the header, in a full PHPStan run we only consider the core and composer directories:

  paths:
    - .
    - ../composer

But if sites/default/default.settings.php is modified, it is analysed during a partial run, because PHPStan is not configured to explicitly exclude it:

$ ../vendor/bin/phpstan analyze ../sites/default/default.settings.php
Note: Using configuration file /var/www/html/drupal/core/phpstan.neon.dist.
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ------------------------------------------- 
  Line   sites/default/default.settings.php         
 ------ ------------------------------------------- 
  662    Variable $app_root might not be defined.   
  662    Variable $site_path might not be defined.  
 ------ -------------------------------------------

Steps to reproduce

Proposed resolution

Extend excludePaths to include default.settings.php:

  excludePaths:
    # Skip settings.
    - ../*/*settings*.php

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#7 3322182-7.patch400 byteslongwave
#2 3322182-2.patch363 byteslongwave

Comments

longwave created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
StatusFileSize
new 3322182-2.patch363 bytes
spokje’s picture

spokje
Primary language Dutch
commented

I think you also have to remove the 2 errors for default.settings.php in the baseline?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

I wonder if instead this should be more specific, ie. ignore ../sites/*/*settings*.php?

catch’s picture

catch
he/him
Primary language English
commented

Or... should we be ignoring the entire sites directory?

spokje’s picture

spokje
Primary language Dutch
commented

+1 to ignoring the /sites directory, there's nothing but config code in there anyway?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Title: Ignore sites/default/default.settings.php in PHPStan » Ignore sites directory in PHPStan
StatusFileSize
new 3322182-7.patch400 bytes

OK, let's try this.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Reviewed & tested by the community

Makes sense to me

spokje’s picture

spokje
Primary language Dutch
commented
Status: Reviewed & tested by the community » Needs work

Still think we have to remove the reported errors for default.settings.php from the baseline: https://git.drupalcode.org/project/drupal/-/blob/10.0.x/core/phpstan-bas...

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review

That is the other copy of the file, used by the scaffolding plugin. We could also exclude that directory, but I think it's probably worth scanning and keeping in the baseline?

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs review » Reviewed & tested by the community

Ah, sorry, you're absolutely right. back to RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

I think you can have code in sites that you might want to scan - but then you should have your own config and baseline so this works for as far as I can see.

Committed and pushed 6a0ea57c28 to 10.1.x and e4deca18fd to 10.0.x. Thanks!

  • alexpott committed 6a0ea57 on 10.1.x 
    Issue #3322182 by longwave, Spokje, catch: Ignore sites directory in...

  • alexpott committed e4deca1 on 10.0.x 
    Issue #3322182 by longwave, Spokje, catch: Ignore sites directory in...

Status: Fixed » Closed (fixed)

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