Remove ExtensionParser::scanDirectory() FileCache usage or share it with InfoParser

alexpott → closed merge request !10376

I think we should use the InfoParser it'll mean less reading of files and shares the info we want. One thought is should we allow the info parser service to passed in? That'd make it simple for the extension.list.* services but it'd complicate things for services constructing an ExtensionDiscovery object that don't already have the info parser.

One thing to note is that extension parser seems more items than info parser.

Based on the slack conversation that sparked this issue it seems to be themes inside uninstalled profiles. Though it was not a rigorous check.

Opened 📌 [meta] Lots of non-plugin PHP classes in plugin directories Active based on the discoveries here.

Will open an issue about the test profiles now, did a bit of extra testing and this definitely feels like a waste.

I think 🐛 Test profiles should be in a tests directory Active explains some of the mis-match, or at least, we 're definitely doing some unnecessary parsing.

Small comments on the MR.

Answered all the comments.

This looks good to me. Do we ever call this code with $use_file_cache = FALSE? Maybe worth a follow-up to deprecate that code path if not.

Yeah we do. We do it when we scan for DB drivers see \Drupal\Core\Extension\DatabaseDriverList::getExtensionDiscovery() - not sure why FWIW. And we do when we generate a theme from a starterkit.- which makes sense to me.

Starterkit theme generation will only happen on the cli, which has (if it's enabled) it's on apcu cache which is not shared between cli processes, so if those are the only two cases I think we could simplify things and ditch it.

I have one nitpick on the MR.
I think the CR needs to be updated for the part: "it will need to change in 12.0.0". We are not doing it in 12.0 but in 11.X.

My question has been answered.
All code changes look good to me.
For me it is RTBC.

Haven't looked at the code. But we can't be ready to commit this if the title and the summary are "Do X or Y". Which is it? 😅

After reading the MR diff, here's a first stab at a better title and summary. Further refinements welcome.

Resolved all the open MR threads. Then closely reviewed the changes. Opened 3 new suggestions, 2 for pedantic nits, one with a real question about the `$use_file_cache` parameter to the constructor.

All my feedback is addressed. I opened 📌 Deprecate the $use_info_parser parameter to ExtensionParser::scanDirectory() Active for the followup proposed in comments 14-16.

Meanwhile, first pass at saving credits:
@alexpott for all the code.
@berdir for the issue write up and various discussions that lead to this.
@smustgrave, @daffie, @catch for reviews.
myself for reviews, metadata cleanup and opening the follow-up so this can be closed and forgotten without loose ends.

Doesn't seem like the comment from @nicxvan qualifies.

Back to RTBC.

Thanks!
-Derek