GeezerVison

Any chance we could turn the else here into a continue while we're at it? (I noticed foundModule only gets used after this)

The method GetModuleForRootedPaths is massively refactored to change the behavior of Get-Module <directory-path> -ListAvailable and Get-Module <directory-path> -ListAvailable -All. The new behvaior will make them the same as if running Get-Module -ListAvailable and Get-Module -ListAvailable -All when $env:PSModulePath == <directory-path>.

Hooray for this and the one above!

Could we give the type here instead of the var? The ternary method call makes it less than obvious.

Fixed.

What's the nature of the change here? Why do we not need to do the duplicate checks now?

The method GetModuleForRootedPaths is massively refactored. Please review again. Thanks!

The formatter for Get-Module -ListAvailable does logic like this, and I think it would be very helpful as a property or method on PSModuleInfo in general. I know we've had to do the same logic in PowerShellEditorServices at some point.

I feel like we ought to factor it into a method. That and I'm just averse to such a large inline delegate.

The OrderBy call was previous OrderBy(m => m.Name). However, simply ordering by name will cause the modules from the same parent folder to be divided by some modules in more nested sub-folders. I have moved this delegate to a method in ModuleUtils.cs.

Maybe String.IsNullOrEmpty is better here?

This method is removed now.

I assume rather than throw this returns null or ""?

The method is removed now.

This method returns an empty string. I changed to use string.IsNullOrEmpty below.

Does this method return a Version?

It returns List<Version>. But the method is removed now.

The logic in this method looks very similar to the Import-Module local load logic:

Where we first check the path, then the folder, then the versions, then the extensions, etc.

Same comments here as above

Add a new line.

How hard is the requirement on names being an array? I only ask because I notice this part of the code tends to have collection types in signatures vary a lot (some IEnumerable<string>, some string[], some List<string>) and we should probably work out a better rationale for what types we prefer.

If there is a particular performance implication from using string[] then that makes sense, but in general I think we should favour the principle of using an interface with only the properties required by each method; if we only want to go through entries sequentially, IEnumerable<T>, if we want to know how many there are up front, something like ICollection<T>, if we want indexing IList<T>.

I'm not making a particular suggestion here I suppose. I just have the impression that the various collection types used in the methods in the module cmdlets weren't used for any particular reason, and we should be more judicious about what collection types we use (and the semantics of each) given how intensively we handle them.

For this method, it makes more sense to use List<string>, so the top-level caller doesn't need to cast the List to a string array. I have updated the code.

The comment above I read as saying "we can skip analysis if all of functions/cmdlets/aliases to export are given".

But I would translate that as the condition:

!needToAnalyzeScriptModules && (sawExportedCmdlets && sawExportedFunctions && sawExportedAliases))

The expression here is equivalent to:

!needToAnalyzeScriptModules && !(sawExportedCmdlets && sawExportedFunctions && sawExportedAliases))

Which I think would be "analysis is not required and any of functions/cmdlets/aliases to export is missing".

Do the variable names mean something converse to how I read them?

The comment above I read as saying "we can skip analysis if all of functions/cmdlets/aliases to export are given".

That's correct.
That means if needToAnalyzeScriptModules == false when we reaching here, we might still change it to true if we didn't see one of "FunctionToExport", "CmdletToExport" and "AliasToExport".

One thing worth to note is the if block here is not doing further analysis, but checking if we really need to do further analysis -- it's changing the value of needToAnalyzeScriptModules to true.

Oh right! The comment is slightly unclear to me because I didn't realise that the if is really a check to see if we maybe do need to do further analysis.

I would have worded it more as "If any of 'FunctionsToExport', 'CmdletsToExport' or 'AliasesToExport' were not given, we must check to see if more analysis is needed". Not saying the wording should be that way, just submitting an alternate description

Good suggestion. I like your comment better. Will update.

A much needed file!

Hopefully, more tests can be added gradually.