Skip to content

test: ProjectCodeTest::testExpectedInputOrder - move checks for testFix... methods#9291

Merged
keradus merged 12 commits intomasterfrom
projectCode2
Dec 14, 2025
Merged

test: ProjectCodeTest::testExpectedInputOrder - move checks for testFix... methods#9291
keradus merged 12 commits intomasterfrom
projectCode2

Conversation

@keradus
Copy link
Copy Markdown
Member

@keradus keradus commented Dec 11, 2025

partially replaces #9289 (comment)

@keradus keradus marked this pull request as ready for review December 11, 2025 22:14
@coveralls
Copy link
Copy Markdown

coveralls commented Dec 11, 2025

Coverage Status

coverage: 93.171%. remained the same
when pulling d7077d5 on projectCode2
into 78eee04 on master.

@keradus keradus changed the title test: \ProjectCodeTest::testExpectedInputOrder\ - more checks for \te…stFix...\ methods test: ProjectCodeTest::testExpectedInputOrder - more checks for testFix... methods Dec 11, 2025
Comment thread tests/AutoReview/ProjectCodeTest.php Outdated
);
}

if (isset($parameterNames[1], $parameterNamesToPosition['input'])) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder which approach we should have (it applies to all param positions/names):

  1. If there is 2nd paramenter, it should be named input (from the other PR).
  2. If there is a parameter named input, it should be at 2nd position (this PR).

I prefer 1 as it would also guarantee no typos in param names, as well as will find this: https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/blob/v3.92.0/tests/Fixer/FunctionNotation/NullableTypeDeclarationForDefaultNullValueFixerTest.php#L346

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.

especially on brought example, totally ok to have expected + configuration, without input.

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.

actually, only one case like that, let me go with your suggestion # 1

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.

applied

# Conflicts:
#	tests/Fixer/FunctionNotation/NullableTypeDeclarationForDefaultNullValueFixerTest.php
#	tests/Test/AbstractFixerTestCase.php
# Conflicts:
#	tests/Test/AbstractFixerTestCase.php
@keradus keradus changed the title test: ProjectCodeTest::testExpectedInputOrder - more checks for testFix... methods test: ProjectCodeTest::testExpectedInputOrder - move checks for testFix... methods Dec 14, 2025
@keradus keradus enabled auto-merge (squash) December 14, 2025 00:05
@keradus keradus merged commit 2d68422 into master Dec 14, 2025
33 checks passed
@keradus keradus deleted the projectCode2 branch December 14, 2025 00:11
stakovicz pushed a commit to stakovicz/PHP-CS-Fixer that referenced this pull request Jan 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants