Skip to content

Fix the regression in tab completing positional parameters - #27857

Merged
Dongbo Wang (daxian-dbw) merged 3 commits into
PowerShell:masterfrom
daxian-dbw:tab-regression
Aug 19, 2026
Merged

Dongbo Wang (daxian-dbw) merged 3 commits into
PowerShell:masterfrom
daxian-dbw:tab-regression

Conversation

@daxian-dbw

@daxian-dbw Dongbo Wang (daxian-dbw) commented Aug 15, 2026

Copy link
Copy Markdown
Member

PR Summary

The PR #17796 and #18755 introduced a regression in tab completion -- dir | % <tab> completes with member names of FileInfo object before that change (v7.2 v7.3 behavior), but after that PR, it completes with file names within the current directory (v7.4 + behavior).

This is because the original code that handled positional parameter completion was able to preserve an alternative non-default-set positional parameter, which it would fall back when the default-set positional parameter had no completion result. For ForEach <tab>, the default-set positional parameter applicable for the position is -Process [ScriptBlock], which has no completion result, so it falls back to the non-default-set parameter -MemberName [string].

However, the PR #17796 + #18755 removed the alternative positional parameter preservation, so for ForEach <tab>, it will complete for -Process [ScriptBlock] only. That caused the regression.

Improvements to positional parameter completion

The implementation before #17796 + #18755 is also inconsistent:

  1. Only 1 non-default-set parameter is preserved. That means if there are more than 1 non-default parameter sets and all contain positional parameter with the same position, only one parameter of them will be considered in completion, depending on the discovery order, and the rest will be ignored.
  2. If we only find a non-default set parameter, then we will not fall back to the remaining-argument parameters no matter that parameter has completion results or not; but if we only find a default-set parameter, we will fall back to the remaining-argument parameters if there is no completion result.

This pull request improves the logic for positional parameter completion in PowerShell. It fixed the regression and those inconsistencies. It keeps the improvement by #17796 + #18755 -- find the positional parameter with the closest position instead of the exact position. In addition, it prioritizes the default-set positional parameter and preserve non-default-set parameters with the same best position.

It refactored the logic in CompletePositionalArgument (in CompletionCompleters.cs) to:

  • Find all parameters at the closest position across valid parameter sets, prioritizing the default set when available, but falling back to alternatives if needed.
  • Continue through alternative parameters for completions if the default-set parameter yields no results. Process all candidates at the best position until a completion is found.
  • Only fall back to remaining-argument parameters when no applicable positional parameter is found.

Enhanced test coverage

Added new tests in TabCompletion.Tests.ps1 to verify:

  • Fallback to positional parameters from alternative parameter sets when the default set has no completions.
  • Iterative attempts across parameter sets until completions are found.
  • Correct completion behavior for parameters declared at different positions in different parameter sets, with and without a default parameter set.

PR Checklist

@daxian-dbw Dongbo Wang (daxian-dbw) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Aug 15, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@daxian-dbw
Dongbo Wang (daxian-dbw) marked this pull request as ready for review August 17, 2026 18:02
@daxian-dbw
Dongbo Wang (daxian-dbw) requested a review from a team as a code owner August 17, 2026 18:02
Copilot AI lite review requested due to automatic review settings August 17, 2026 18:02
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a regression in positional-parameter tab completion introduced by earlier changes to positional argument selection/prioritization. It restores the ability to try positional parameters from alternative parameter sets when the default parameter set’s positional parameter yields no completions, while keeping the “closest position” behavior.

Changes:

  • Refactors CompletePositionalArgument to track all best-position candidates across valid parameter sets, trying the default-set candidate first and then alternatives until a completion is produced.
  • Adjusts fallback behavior so “remaining arguments” parameters are only considered when no applicable positional parameter is found.
  • Adds tests to validate fallback across parameter sets and correct behavior when positions differ between parameter sets (with/without a default set).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Refactors positional argument candidate discovery and processing order (default set first, then alternative sets), and tightens fallback to remaining-argument parameters.
test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 Adds coverage for positional-parameter fallback across parameter sets and scenarios with differing positional declarations across sets.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@iSazonov Ilya (iSazonov) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Many thanks!

Have you plans to backport?

if (defaultSetParam is not null)
{
ProcessParameter(commandName, commandAst, context, result, defaultSetParam, boundArguments);
if (result.Count > 0)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the result.Count could be 1 here where result[0] could be CompletionResult.Null. In that case we should continue down the line and not return.

@daxian-dbw Dongbo Wang (daxian-dbw) Aug 19, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CompletionResult.Null is an indicator that means stopping subsequent fallback. We should follow the existing behavior -- only fall back when there is no result.

@daxian-dbw
Dongbo Wang (daxian-dbw) merged commit f48794c into PowerShell:master Aug 19, 2026
37 checks passed
@daxian-dbw
Dongbo Wang (daxian-dbw) deleted the tab-regression branch August 19, 2026 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants