Squiz/ObjectInstantiation: move parse error test to its own file

[biinari]

Description

Suggested changelog entry

N/A

Related issues/external references

See #143

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
  • This change is only breaking for integrators, not for external standards or end-users.
• Documentation improvement

PR checklist

• I have checked there is no other PR open for the same change.
• I have read the Contribution Guidelines.
• I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• I have added tests to cover my changes.
• I have verified that the code complies with the projects coding standards.
• [Required for new sniffs] I have added XML documentation for the sniff.

[jrfnl]

In principle all good, but if I look at the original commit (PR squizlabs/PHP_CodeSniffer#836) which introduced this, I'm not sure this test was intended as a parse error test, nor that it has value for this sniff.

I think this test has been superseded by PR
squizlabs/PHP_CodeSniffer#3484, which also added dedicated tests for that part of the Tokenizer, so instead, it might be better to remove this test from the ObjectInstantiation test case file.

@biinari What do you think ?

[biinari]

Well spotted @jrfnl, on closer inspection I agree that this test was not intended to check for parse errors / live coding.

It looks like it was intended to check for new when used as a name rather than a keyword (though it may have failed to cover that given the line finishes early causing a parse error). This test for keyword context does indeed seem to be covered by squizlabs/PHP_CodeSniffer#3484 and squizlabs/PHP_CodeSniffer#3608, which added some tests for function names.

I can't see any way that this sniff would need a live coding test case, all the needed context comes before the new keyword so it would not create a false positive.

I'm happy to close this PR and submit another to remove this test from the test case file without the file rename.

[jrfnl]

I can't see any way that this sniff would need a live coding test case, all the needed context comes before the new keyword so it would not create a false positive.

Just checked the sniff code and I agree with that conclusion.

I'm happy to close this PR and submit another to remove this test from the test case file without the file rename.

Sounds like a good plan!