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

Constant visibility false positive on non-class constants #2615

Closed
dereuromark opened this issue Sep 27, 2019 · 7 comments
Closed

Constant visibility false positive on non-class constants #2615

dereuromark opened this issue Sep 27, 2019 · 7 comments
Milestone

Comments

@dereuromark
Copy link
Contributor

dereuromark commented Sep 27, 2019

 3 | WARNING | Visibility must be declared on all constants if your
   |         | project supports PHP 7.1 or later

But the code is

<?php

const APPLICATION_ENV = 'development';

so a normal classless file, as such visibility for such a const cannot be given, can it?

@jrfnl
Copy link
Contributor

jrfnl commented Sep 27, 2019

Confirmed. This is a bug.

@Beakerboy
Copy link

What Is the resolution on this going to look like? I have a project that is advertised as supporting php 7.0+. Class constants just started being flagged with this today on Travis-ci. Since visibility on constants didn’t appear until 7.1, do I need to turn this check off in my coding standards file?

@gmta
Copy link

gmta commented Sep 29, 2019

For completeness, the sniff is PSR12.Properties.ConstantVisibility.NotFound.

@gsherwood gsherwood added this to the 3.5.1 milestone Sep 30, 2019
@gsherwood gsherwood changed the title Const false positive on visibility Constant visibility false positive on non-class constants Oct 1, 2019
@gsherwood
Copy link
Member

Thanks for reporting this. Fix will be in 3.5.1, which will be released when I've fixed the other PSR-12 issues that have come through. Check out the milestone for the list.

@vaso123
Copy link

vaso123 commented Oct 27, 2020

Not really fixed. It is also in: PHP_CodeSniffer version 3.5.8 (stable) by Squiz (http://www.squiz.net) with PHPStorm. I am using PSR-12

EDIT: This is a class constant.

const

@jrfnl
Copy link
Contributor

jrfnl commented Oct 27, 2020

@vaso123 I think you misunderstand the earlier reported issue. Constant visibility was previously - incorrectly - demanded for non-class constants. That bug has been fixed and can no longer be reproduced.

I suspect you don't want to use class constant visibility as your project needs to support older PHP versions. Is that correct ?
In that case, you will need to ignore the rule from within your custom ruleset or from the command line.
This is not a bug. PSR12 demands visibility for class constants. You can choose not to abide by it.

@vaso123
Copy link

vaso123 commented Oct 27, 2020

Ok, got it, you are right, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants