BUG: Package Functions #242

MGatner · 2021-11-29T13:45:54Z

Describe the bug

Composer Unused does not seem to detect a package as a dependency if the source repo uses a function that is defined in that package.

Error dump

Live example in Actions output: https://github.com/tattersoftware/codeigniter4-files/runs/4347265699?check_suite_focus=true

Run composer-unused -vvv --profile --ansi --no-interaction --no-progress --excludePackage=php
Running 2.1.12 (2021-11-09 16:02:04) with PHP 8.0.13 on Linux / 5.11.0-1021-azure

Loading packages
----------------
 ! [NOTE] Found 7 package(s) to be checked.                                     
Scanning files from basedir /home/runner/work/codeigniter4-files/codeigniter4-files
-----------------------------------------------------------------------------------

Results
-------
Found 4 used, 3 unused and 5 ignored packages

 Used packages
 ✓ tatter/exports
 ✓ tatter/permits
 ✓ tatter/settings
 ✓ tatter/thumbnails
[18.7MiB/0.38s] Memory usage: 18.68MiB (peak: 18.97MiB), time: 0.38s

 Unused packages
 ✗ tatter/alerts
 ✗ tatter/assets
 ✗ tatter/audits

 Ignored packages
 ○ php (Package excluded by cli/config)
 ○ components/jquery (Package provides no namespace)
 ○ enyo/dropzone (Package provides no namespace)
 ○ fortawesome/font-awesome (Package provides no namespace)
 ○ twbs/bootstrap (Package provides no namespace)
Error: Process completed with exit code 1.

In this example tatter/alerts defines a helper function alerts() (https://github.com/tattersoftware/codeigniter4-alerts/blob/develop/src/Helpers/alerts_helper.php) which is used by the source repo's controller (https://github.com/tattersoftware/codeigniter4-files/blob/0ee8595a60394cebeb42ab11a3e095042b0ebd5e/src/Controllers/Files.php#L374).

The text was updated successfully, but these errors were encountered:

icanhazstring · 2021-11-29T13:47:23Z

Hi @MGatner.

Thanks for reaching out, again :)
Can you validate this against dev-master as I redesigned the ability to detect usages of functions and constants for next 0.8 release

MGatner · 2021-11-29T14:19:37Z

Thanks! Checking now...

MGatner · 2021-11-29T14:27:31Z

Actually detecting less now.

Running 2.1.12 (2021-11-09 16:02:04) with PHP 7.4.10 on Linux / 5.11.0-1020-aws

Results
-------

Found 4 used, 8 unused and 0 ignored packages

 Used packages
 ✓ php
 ✓ tatter/exports
 ✓ tatter/settings
 ✓ tatter/thumbnails

 Unused packages
 ✗ components/jquery
 ✗ enyo/dropzone
 ✗ fortawesome/font-awesome
 ✗ tatter/alerts
 ✗ tatter/assets
 ✗ tatter/audits
 ✗ tatter/permits
 ✗ twbs/bootstrap

 Ignored packages

The asset repos are expected (they used to be "Package provides no namespace") but tatter/permits was "used" before and now is "unused", and same issue with tatter/alerts.

icanhazstring · 2021-11-29T14:51:57Z

Ok, I have removed some checks for the packages to be scanned.

Now there are some "new" packages delivered using composer.

Some packages are delivered using composer, which are no composer package at all. These are npm packages. This can maybe be removed if there is no autoloading at all. (Will be fixed)
tatter/alerts (and the others) are special ones, these should be recognized but somehow the usage is not found. Maybe even the alert symbol (and others) for those packages are not found. Will check on that.

MGatner · 2021-11-29T16:54:12Z

Thanks! Let me know how I can help.

icanhazstring · 2021-11-29T17:54:17Z

Will create a PR on composer-unused/symbol-scanner where I will add some test cases.

Would be nice if you could review them as I try to replicate the issue with your packages.

icanhazstring · 2021-11-30T17:22:08Z

Great news @MGatner I could replicate the behavior for this issue.
So I am working on a fix with the mentioned issue above. Will keep you posted.

Problem is:
The usage with use function xxx is already covered as a used symbol.
Problem is that <function>() as a function call is not covered right now. So I will add an update to composer-unused/symbol-parser to cover this missing feature :)

MGatner · 2021-12-01T02:00:53Z

That's awesome! Thank you for the update.

icanhazstring · 2021-12-16T12:16:47Z

Hi @MGatner could the check out the PR and see if this is working for you?
I did my best to replicate your issue :)

MGatner · 2021-12-17T02:57:03Z

@icanhazstring It still seems to be missing some of them. In the same example above, I'm looking at the tatter/alerts package which houses the function alert(), used in FilesController.php multiple times.

I will say, the new version is waaaay faster! Very impressively so. Let me know if there are debug logs or something I can provide to help.

icanhazstring · 2021-12-17T08:09:36Z

Thanks, will take a deeper look

icanhazstring · 2022-01-04T16:11:43Z

I found the problem with this package.
Problem is that the alert function is coming from a file not containing the psr-4 namespace.
https://github.com/tattersoftware/codeigniter4-alerts/blob/develop/src/Helpers/alerts_helper.php

While the composer.json is only defining the psr-4 autoload.
If there would be a files section in autoload containing the function, this would work.

    "autoload": {
        "psr-4": {
            "Tatter\\Alerts\\": "src"
        },
        "files": [
            "src/Helpers/alerts_helper.php"
        ],
        "exclude-from-classmap": [
            "**/Database/Migrations/**"
        ]
    },

Or you can add the namespace of to the function helper itself, this way it would be found using a NamespaceSymbol.

MGatner · 2022-01-04T16:21:41Z

Okay, I will try adding it to Composer's files and report back. This is a common issue with CodeIgniter Helper files - for example, in PHPStan we supply a list of additional files to scan: https://github.com/tattersoftware/codeigniter4-files/blob/develop/phpstan.neon.dist

icanhazstring · 2022-01-04T16:24:00Z

I see. If you add them to autoload.files this should work with phpstan as well.

icanhazstring · 2022-01-04T17:23:49Z

I merged the PR into main.
So if you want, you can try it using the dev-main version.

MGatner · 2022-01-04T19:29:29Z

Okay, that worked, but also one weirdness! I pulled the latest and used it on codeigniter4/files and it missed the "helper files only" packages, as expected:

Results
-------

Found 4 used, 4 unused and 0 ignored packages

 Used packages
 ✓ php
 ✓ tatter/exports
 ✓ tatter/frontend
 ✓ tatter/permits

 Unused packages
 ✗ enyo/dropzone
 ✗ tatter/alerts
 ✗ tatter/preferences
 ✗ tatter/thumbnails

 Ignored packages

Then I modified vendor/tatter/alerts/composer.json to simulate having added the files as you suggested:

    "autoload": {
        "psr-4": {
            "Tatter\\Alerts\\": "src"
        },
        "exclude-from-classmap": [
            "**/Database/Migrations/**"
        ],
        "files": [
            "src/Helpers/alerts_helper.php"
        ]
    },

After running composer dump when I rerun composer-unused it detects tatter\alerts as used (yay!) but now also detects tatter\preferences and tatter\thumbnails, neither of which are used by Alerts!

Results
-------

Found 7 used, 1 unused and 0 ignored packages

 Used packages
 ✓ php
 ✓ tatter/alerts
 ✓ tatter/exports
 ✓ tatter/frontend
 ✓ tatter/permits
 ✓ tatter/preferences
 ✓ tatter/thumbnails

 Unused packages
 ✗ enyo/dropzone

 Ignored packages

Something fishy going on here?

icanhazstring · 2022-01-04T20:10:58Z

Funny 😅
Let me check in quick example why this is the case.

icanhazstring · 2022-01-04T21:04:37Z

Tried to reproduce with only tatter/alerts and tatter/thumbnails while only alert in use.
I couldn't install all packages at once you are using. in dev-developthey conflict with one another.

Can you give me the precise version constraint for each package you are using?

Results
-------

Found 1 used, 1 unused, 0 ignored and 0 zombie packages

 Used packages
 ✓ tatter/alerts

 Unused packages
 ✗ tatter/thumbnails

 Ignored packages

MGatner · 2022-01-05T15:25:51Z

Here are the exact steps to recreate it. I just did this in a fresh environment so I know it isn't cache or something.

git clone https://github.com/tattersoftware/codeigniter4-files.git
cd codeigniter4-files/
composer update
Run composer-unused and verify the following packages fail:
- tatter/alerts
- tatter/preferences
- tatter/thumbnails
vi vendor/tatter/alerts/composer.json
Add the files section to autoload (can copy from above)
Rerun composer-unused and verify the same packages now pass.

icanhazstring · 2022-01-05T16:15:58Z

Thanks. Will check it out

icanhazstring · 2022-01-06T11:14:14Z

PHP: 7.4
Composer: 2.2.3

Before autoload fix:

± |develop U:1 ✗| → ../../composer-unused/composer-unused/bin/composer-unused 


Results
-------

Found 4 used, 4 unused, 0 ignored and 0 zombie packages

 Used packages
 ✓ php
 ✓ tatter/exports
 ✓ tatter/frontend
 ✓ tatter/permits

 Unused packages
 ✗ enyo/dropzone
 ✗ tatter/alerts
 ✗ tatter/preferences
 ✗ tatter/thumbnails

 Ignored packages

 Zombies exclusions (did not match any package))

After outload fix:

± |develop U:1 ✗| → ../../composer-unused/composer-unused/bin/composer-unused 


Results
-------

Found 5 used, 3 unused, 0 ignored and 0 zombie packages

 Used packages
 ✓ php
 ✓ tatter/alerts
 ✓ tatter/exports
 ✓ tatter/frontend
 ✓ tatter/permits

 Unused packages
 ✗ enyo/dropzone
 ✗ tatter/preferences
 ✗ tatter/thumbnails

 Ignored packages

 Zombies exclusions (did not match any package))

Looks good to me though?

MGatner · 2022-01-06T13:13:50Z

😳 That's bizarre. Same steps as mine? I've recreated this twice, and every time it erratically considers "thumbnails" and "preferences" in use.

Maybe environment difference? All my tests are on Ubuntu 20.04 with PHP 7.4, not sure the precise Composer but version 2.x

icanhazstring · 2022-01-06T16:29:13Z

I am working on Mac though. Maybe I'll setup a docker with Ubuntu. But that seems odd 🤔

MGatner · 2022-01-07T13:03:41Z

I mean, technically it is correct. It is just odd that it picks up the helper file functions without their addition to Composer's files array. I'm fine to roll with it for now and just keep an eye on changes.

FWIW for some packages with helper files I will probably opt to ignore them rather than have them autoloaded every time; in some scenarios that is quite useful but in others the helper function are mostly "optional" and rarely used. It's too bad there's not an "autoload-dev" files section. If you every consider adding external Config files similar to PHPStan or CS Fixer, this would be on my request: additional files to load before scanning.

icanhazstring · 2022-01-07T13:43:52Z

Feel free to add a feature request discussion 😉

So we can close this one for now?

icanhazstring added the bug Something isn't working label Nov 29, 2021

icanhazstring mentioned this issue Nov 30, 2021

Feature: Parse FunctionCall usage composer-unused/symbol-parser#2

Closed

icanhazstring mentioned this issue Dec 16, 2021

Check for function invocation parsing #249

Merged

MGatner closed this as completed Jan 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Package Functions #242

BUG: Package Functions #242

MGatner commented Nov 29, 2021

icanhazstring commented Nov 29, 2021 •

edited

MGatner commented Nov 29, 2021

MGatner commented Nov 29, 2021

icanhazstring commented Nov 29, 2021 •

edited

MGatner commented Nov 29, 2021

icanhazstring commented Nov 29, 2021

icanhazstring commented Nov 30, 2021 •

edited

MGatner commented Dec 1, 2021

icanhazstring commented Dec 16, 2021

MGatner commented Dec 17, 2021

icanhazstring commented Dec 17, 2021

icanhazstring commented Jan 4, 2022 •

edited

MGatner commented Jan 4, 2022

icanhazstring commented Jan 4, 2022

icanhazstring commented Jan 4, 2022 •

edited

MGatner commented Jan 4, 2022

icanhazstring commented Jan 4, 2022

icanhazstring commented Jan 4, 2022 •

edited

MGatner commented Jan 5, 2022

icanhazstring commented Jan 5, 2022

icanhazstring commented Jan 6, 2022

MGatner commented Jan 6, 2022

icanhazstring commented Jan 6, 2022

MGatner commented Jan 7, 2022

icanhazstring commented Jan 7, 2022

BUG: Package Functions #242

BUG: Package Functions #242

Comments

MGatner commented Nov 29, 2021

Describe the bug

Error dump

icanhazstring commented Nov 29, 2021 • edited

MGatner commented Nov 29, 2021

MGatner commented Nov 29, 2021

icanhazstring commented Nov 29, 2021 • edited

MGatner commented Nov 29, 2021

icanhazstring commented Nov 29, 2021

icanhazstring commented Nov 30, 2021 • edited

MGatner commented Dec 1, 2021

icanhazstring commented Dec 16, 2021

MGatner commented Dec 17, 2021

icanhazstring commented Dec 17, 2021

icanhazstring commented Jan 4, 2022 • edited

MGatner commented Jan 4, 2022

icanhazstring commented Jan 4, 2022

icanhazstring commented Jan 4, 2022 • edited

MGatner commented Jan 4, 2022

icanhazstring commented Jan 4, 2022

icanhazstring commented Jan 4, 2022 • edited

MGatner commented Jan 5, 2022

icanhazstring commented Jan 5, 2022

icanhazstring commented Jan 6, 2022

MGatner commented Jan 6, 2022

icanhazstring commented Jan 6, 2022

MGatner commented Jan 7, 2022

icanhazstring commented Jan 7, 2022

icanhazstring commented Nov 29, 2021 •

edited

icanhazstring commented Nov 29, 2021 •

edited

icanhazstring commented Nov 30, 2021 •

edited

icanhazstring commented Jan 4, 2022 •

edited

icanhazstring commented Jan 4, 2022 •

edited

icanhazstring commented Jan 4, 2022 •

edited