Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PSR12] Add failing test for valid code #2617

Closed
wants to merge 1 commit into from

Conversation

duncan3dc
Copy link
Contributor

More if an issue than a pull request sorry.

The following code is valid as per PSR-12, however phpcs reports the following issues:

------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 2 LINES
------------------------------------------------------------------------------------------------------
 2 | ERROR | [ ] The file header must be the first content in the file
 6 | ERROR | [ ] The file-level docblock must follow the opening PHP tag in the file header
 6 | ERROR | [x] Header blocks must be followed by a single blank line

It recognises the inline docblock as a file header, which is incorrect. The fix for this is simple, however that affected the check for leading blank lines in FileHeaderUnitTest.2.inc.

The code for the opening PHP tag also doesn't seem to be correct, PSR-12 only talks about mixed HTML and PHP files, it doesn't require the opening tag to be the first thing in the file in all scenarios (eg the hashbang seen here is valid)

@dereuromark
Copy link
Contributor

Refs #2616

@gsherwood gsherwood added this to the 3.5.1 milestone Sep 30, 2019
@gsherwood
Copy link
Member

gsherwood commented Oct 3, 2019

The code for the opening PHP tag also doesn't seem to be correct, PSR-12 only talks about mixed HTML and PHP files, it doesn't require the opening tag to be the first thing in the file in all scenarios (eg the hashbang seen here is valid)

To PHP, everything outside PHP tags is T_INLINE_HTML. I can't really detect that the content is actually HTML without trying to parse it. It could very well be XML, JSON, YAML, whatever. PSR-12 still wants the file header up the top in these cases.

What I will try and do is just allow hashbang lines as these are very obviously not HTML and have to be up top.

The other issue in this PR is being addressed by issue #2626.

@gsherwood
Copy link
Member

I've allowed the hashbang line and also fixed the issue with the @var comment. I'm closing this as your sample code now passes, and I've included it in a test even though the PR wouldn't merge due to other changes.

Thanks a lot for this report.

@gsherwood gsherwood closed this Oct 3, 2019
@duncan3dc duncan3dc deleted the file-header-issue branch October 4, 2019 09:06
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