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

RedundantCondition for config constants #2076

Closed
iluuu1994 opened this issue Aug 27, 2019 · 6 comments
Closed

RedundantCondition for config constants #2076

iluuu1994 opened this issue Aug 27, 2019 · 6 comments

Comments

@iluuu1994
Copy link
Contributor

Many of our applications look like this:

class Config {
  const DB_DRIVER = 'pdo_sqlsrv'; // Yes, we use sqlsrv :'(
}

if (Config::DB_DRIVER === 'pdo_sqlsrv') {
  // ...
}

ERROR: RedundantCondition - 7:5 - Found a redundant condition when evaluating Config::DB_DRIVER and trying to reconcile type 'string(pdo_sqlsrv)' to string(pdo_sqlsrv)

https://psalm.dev/r/3ed817ceba

I'm aware this isn't great. Constant shouldn't contain variable values. Either way, PHPStan allows defining constants as "dynamic": phpstan/phpstan#1083

Do you see any value in this?

@muglug
Copy link
Collaborator

muglug commented Aug 27, 2019

Why not use a public static const: https://psalm.dev/r/ebd07a3425?

@iluuu1994
Copy link
Contributor Author

@muglug Yeah that would probably be better but unfortunately this is something I just can't change. Anyway, I'll just close this. Psalm shouldn't have to deal with badly designed codebases.

@muglug
Copy link
Collaborator

muglug commented Aug 27, 2019

Psalm shouldn't have to deal with badly designed codebases.

Well then nobody would use it

@iluuu1994
Copy link
Contributor Author

Truth is this is horrible for testability and reusability. I'd much rather it weren't this way in our application but it's not something I can change easily.

Your config values should probably be coming from a separate file and be distributed through dependency injection or something like similar.

@bugreportuser
Copy link
Contributor

I don't even see this being useful just for config constants. Most times, the value of a constant doesn't matter. So, if your application breaks when the value of a constant changes to a different value of the same type, that can be a problem.

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

No branches or pull requests

3 participants