Overview

#3589083: Remove `\Drupal\canvas\CodeComponentDataProvider::getRequiredCanvasDataLibraries` is deleting dead code. Cool!

But why do this manually if it can be automated?

Proposed resolution

Adopt shipmonk-rnd/dead-code-detector and implement usage providers as necessary.

Overview:

  • Initial adoption, zero config: 467 problems reported. But these include many false positives! ⚠️ So we need to provide usage providers (which are AI-generated) to avoid these false positives
  • Custom usage provider for test properties like $modules: 299
  • Custom usage provider for Drupal hooks: 226
  • Refine the usage provider for test properties to look at more: 206
  • Custom usage provider for Canvas-specific test properties: 204
  • Custom usage provider for test methods like ::setDatabaseDumpFiles: 182
  • Custom usage provider for polymorphic overrides: 180
  • Custom usage provider for routing (controllers + access checks): 128
  • … and more

⇒ eventually, no more false positives. So then manually reviewed and fixed them (+9,-226 🪦 200 dead lines deleted!):

  1. https://git.drupalcode.org/project/canvas/-/merge_requests/1081/diffs?co...
  2. https://git.drupalcode.org/project/canvas/-/merge_requests/1081/diffs?co...

User interface changes

None.

Issue fork canvas-3589128

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

wim leers created an issue. See original summary.

wim leers opened merge request !1081

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Followed the instructions at https://github.com/shipmonk-rnd/dead-code-detector and got stuck. Turns out we don't need 

includes:
    - vendor/shipmonk/dead-code-detector/rules.neon

… and that wasted my time 😬

Seems like this is working, here's an excerpt from my local dev env, hopefully CI confirms:

 ------ ------------------------------------------------------------------------------------------------------- 
  Line   tests/src/Kernel/PropExpressionKernelTest.php                                                          
 ------ ------------------------------------------------------------------------------------------------------- 
  65     Property Drupal\Tests\canvas\Kernel\PropExpressionKernelTest::$modules is never read                   
         🪪  shipmonk.deadProperty.neverRead                                                                    
  75     Property Drupal\Tests\canvas\Kernel\PropExpressionKernelTest::$configSchemaCheckerExclusions is never  
         read                                                                                                   
         🪪  shipmonk.deadProperty.neverRead                                                                    
 ------ ------------------------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------------------------ 
  Line   tests/src/Kernel/PropShapeRepositoryTest.php                                                          
 ------ ------------------------------------------------------------------------------------------------------ 
  70     Property Drupal\Tests\canvas\Kernel\PropShapeRepositoryTest::$configSchemaCheckerExclusions is never  
         read                                                                                                  
         🪪  shipmonk.deadProperty.neverRead                                                                   
 ------ ------------------------------------------------------------------------------------------------------ 

 ------ ------------------------------------------------------------------------------------------- 
  Line   tests/src/Kernel/PropSource/PropSourceTestBase.php                                         
 ------ ------------------------------------------------------------------------------------------- 
  49     Property Drupal\Tests\canvas\Kernel\PropSource\PropSourceTestBase::$modules is never read  
         🪪  shipmonk.deadProperty.neverRead                                                        
 ------ -------------------------------------------------------------------------------------------

wim leers changed the visibility of the branch 3556426-computedentitycanonicalrelativeurl-breaks-for to hidden.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

🥳 It does report #3589083: Remove `\Drupal\canvas\CodeComponentDataProvider::getRequiredCanvasDataLibraries`'s

 ------ ----------------------------------------------------------------------- 
  Line   src/CodeComponentDataProvider.php                                      
 ------ ----------------------------------------------------------------------- 
  176    Unused                                                                 
         Drupal\canvas\CodeComponentDataProvider::getRequiredCanvasDataLibrari  
         es                                                                     
         🪪  shipmonk.deadMethod                                                
 ------ -----------------------------------------------------------------------

— https://git.drupalcode.org/project/canvas/-/jobs/9725078#L1102

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: PHPStan: evaluate shipmonk-rnd/dead-code-detector » PHPStan: adopt shipmonk-rnd/dead-code-detector
Issue tags: +AI-accelerated

Currently letting claude figure out how to fix the many false positives without just adding ignores.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes

(Using AI for all the usage providers, only initial commit is 100% human.)

Eliminated most false positives: down to 128 errors, but there's still some false positives in there.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes

Yay! As of https://git.drupalcode.org/project/canvas/-/merge_requests/1081/diffs?co..., this is now green with the exception of what #3589083 just fixed.

200 dead lines deleted! Overview in the issue summary.

First self-review, only then letting others review.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: PHPStan: adopt shipmonk-rnd/dead-code-detector » Automate review using PHPStan: adopt shipmonk-rnd/dead-code-detector
Assigned: wim leers » Unassigned
Priority: Normal » Major
Issue summary: View changes
Status: Active » Needs review

All AI-generated stuff lives in phpstan_rules/.

Everything outside that was done manually.

  Total cost:            $31.06                                                                                                                                                                                      
  Total duration (API):  2h 19m 16s                                                                                                                                                                                  
  Total duration (wall): 6h 14m 23s                                                                                                                                                                                  
  Total code changes:    2029 lines added, 562 lines removed                                                                                                                                                         
  Usage by model:                                                                                                                                                                                                    
      claude-haiku-4-5:  6.7k input, 4.1k output, 22.5k cache read, 9.9k cache write ($0.0420)                                                                                                                       
     claude-sonnet-4-6:  699.7k input, 447.7k output, 50.5m cache read, 1.9m cache write ($31.02)

$31.06 + a few hours of my time to never have to chase dead code again seems well worth it :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Category: Task » Feature request
Status: Needs review » Reviewed & tested by the community

  • wim leers committed f9a85910 on 1.x 
    feat(Project management): #3589128 Automate review using PHPStan: adopt...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented

👏🏽👏🏽👏🏽