Check functions from file autoloading

[JoshuaBehrens]

Describe the bug

A function from a composer dependency is used but its usage is not detected. I assume it is about a function that is provided via files autoloading: https://github.com/bpolaszek/php-iterable-functions/blob/0cc66fecec909546752b67215af72acbb66445e3/composer.json#L10

Additional information

{
    "require": {
        "php": ">=7.4",
        "bentools/iterable-functions": ">=1.4"
    },
    "autoload": {
        "psr-4": {
            "FooBar\\": "src/"
        }
    },
    "require-dev": {
        "icanhazstring/composer-unused": ">=0.7"
    }
}

[icanhazstring]

Thanks for reporting. Yes you are correct. Currently the only thing that is being analysed from dependencies are namespaces from psr-4 and psr-0. Everything else is not checked.

But that would ofc make a lot of sense when it comes to packages that only provide functions instead of class symbols, which in return won't be provided as psr autoload configs but instead of files.

I guess we can mark this as a new feature. I'll check and see if I can gather some information about how to tackle this and put the information into this issue. If you or someone else might try to add it 😉

[JoshuaBehrens]

Although I am thrilled to tackle this I am currently more into using this to support my own projects. Let's see who has time for it first 🚀

[icanhazstring]

Alright so, some thoughts about how to get this working.

1. I will slightly rework how the usages are scanned. Right now everything works against a namespace with a slight "workaround" for core extensions. These are already using kind of a symbol based comparison
2. That being said: I will have to implement a symbol based matching

What is a symbol:
A symbol might be one of three things

• A Class (including the Full Qualified Name)
• A function (might be inside a namespace as well)
• A constant (also maybe inside a namespace)

Using this, rather than checking if a package only provides a namespace I scan all provided symbols by a package against used symbols having used inside my own code.

[JoshuaBehrens]

Just having a namespace check explains the fast execution if this tool. I must admit I thought it already is a symbol-based check 😅

So we need a collection of everything in the fully qualified name that is provided by a package. I think your list of symbols is fine but I like to add two points to the discussion:

• I assume that class includes traits and interfaces, right?
• Global variables... Discussable pattern but still a feature of the language. Do you want to support this?

[icanhazstring]

Funny side node. I don't do a full scan of your code and the package code. I just read the namespace, which includes most of class and constants, then just iterating your files stopping at the first usage.

This should be the handling afterwards still. Since the tool is pretty fast, I want to leave it that way, even checking all symbols.

There a still some shitty implementations which can be optimized. And I should optimize them to keep the speed 😉

Global constants are covered through the php core extension already. So the "only" thing needed is a full scan of composer.autoload.files of the required package to get constant or function symbols

[JoshuaBehrens]

Other analysis tools have optional caching features. Although caching things can be a statement of writing imperformant code it will still improve a second run in any way. In my case I already use analysis caching so I am prepared for this.

I don't call for global constants. I am talking about global variables.

function baz() {
    global $foobar;
}

function yikes() {
    global $foobar; // same object as in line 2
}

But a file can still define global constants... Oh no... the define and is_defined functions... ARGH

[icanhazstring]

Yes. I would probably stick to the same logic as the php extension system works if you call new ReflectionExtension($extension); where you can call getConstants(), getClassNames() or getFunctions().

Every other thing could be an addition for the future if someone needs this.

With the caching I am on the "cache if necessary". IMO designing the system with caching already in mind will lead to inperformant implementations.

---

With that said. One thing with performance in mind would be to change the "list" we are checking against based on the structure of the required package.

E.g. If the package provides a psr0/4 namespace we could check class names first using the namespace. If it provides only files, check for function or constant first. This way we might skip some iterations 😉

[JoshuaBehrens]

I have the feeling I will stumble upon these edge cases 😁 but yes we don't have to over-engineer it all at once.

Caching has to be opt-in, yes.

That is a good thought. I didn't think of that kind of optimization and I share the same expectation that a file autoloader less often publishes classes.

[icanhazstring]

I opened a draft PR (#89) adding the new implementation so you can track the progress.

[JoshuaBehrens]

nice 🕶️ I will test my cases

[icanhazstring]

Hold your horses 😄
There is still only the new implementation besides the old stuff. Need to wire it up if everything is in place :)
Just wanted to let you know. Maybe you see something that I miss in the new implementation.

[JoshuaBehrens]

I got that it is not ready. Just wanted to state that I have test cases to provide (and will for testing purposes).

[icanhazstring]

Just realized while adding the implementation:

Moving autoload.files up front if only file autoload is provided doesn't matter. Using generators the symbols for psr0/4 are simply skipped. So there is no advantage changing the order.

Also function and constant parsing is IO heavy since I have to get the contents of the provided files. So this will always be done last and only if the psr0/4 namespace does not get used beforehand.

[icanhazstring]

I got that it is not ready. Just wanted to state that I have test cases to provide (and will for testing purposes).

Hi @JoshuaBehrens
I think the PR #89 is now in a good state to be tested.

I have added an --experimental|-x option to the composer unused command which will then perform the new symbol scanning. As you mentioned to be willing to provide some real-life testing, would you mind testing it out?

[JoshuaBehrens]

Hi @icanhazstring I put it on my todo list :) So the latest release has this feature already? Or do I have to install the branch locally?

[icanhazstring]

You need to install it using composer require icanhazstring/composer-unused:dev-feature/symbol-scanning.
The latest release was a bugfix release.

The next release having this feature will be 0.8.

[icanhazstring]

This should be solved in the current 0.8.x-dev version.

[icanhazstring]

Closing this. If it still doesn't work for you. Feel free to reopen 👍

[JoshuaBehrens]

Sorry for not actively checking this. I assume it'll work 😅 bet I'll check in when there is a need for changes