Open eileenmcnaughton opened 5 years ago
Good spot @eileenmcnaughton. @prondubuisi It looks like due to the cross-dependencies across core and other modules, we are going to end up polluting the output of the Psalm scans with missing file errors.
I wonder if we could fix this by working from the core => module. So instead of forking each module repo as previously discussed, we fork civi core, pull in incrementally the modules we want to analyse and then specify these modules as the source_dir argument when running Psalm.
So the reason NOT to run just against all of core is that we assume the process will be too big & crash the server - could try it though just in case...
@eileenmcnaughton @jackgleeson I will be running Psalm on the core Folder tomorrow, I guess its time we face the Elephant in the room @eileenmcnaughton do you mind if I copy you in my daily standup mail with @jackgleeson
@prondubuisi yes, please include me!
The elephant might yet turn out to be a woolly mammoth!
@eileenmcnaughton we can specify the paths to scan via the source_dir argument when running Psalm which that should allow us to skip core and just analyse the files we want without running into the missing dependencies problems, as composer should have added all the core files to its autoloader by then.
@jackgleeson will that be enough? The api4 folder specifically calls a number of core functions - e.g above it's looking because the class 'extends CRM_Core_Page ' which is in core
@eileenmcnaughton I wonder if it makes more sense to think of this in unit test terms, e.g. the API4 code is the object/code-under-test and the core code is the collaborator code? I'm not sure how Psalm treats inheritance trees and calls to other code not specified in the source_dir. My first impressions were that it just scans file contents so we should be ok.
I guess we could test to confirm the above
Yep - I guess some trial & error...
Hey @prondubuisi - great to see some results.
Looking at the apiv4 results - https://pastebin.com/VcRVjN3m I see lots of stuff like
I wonder how you are supposed to deal with a dependency on civicrm-core. It might be worth digging into that at this stage & worrying less about other repos
@jackgleeson