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

feat: detect usage of env() function outside of config folder #1828

Merged
merged 2 commits into from
Feb 11, 2024

Conversation

calebdw
Copy link
Contributor

@calebdw calebdw commented Feb 7, 2024

  • Added or updated tests
  • Documented user facing changes

Closes #1695.

Changes

Hello!

This PR adds a rule to report any env calls outside the config directory. Currently it just checks for a config directory in the file name where the call is located. A more robust solution might be to use the container to grab the configured config path? However this is more complicated and I wasn't sure how exactly to do such a thing.

Thanks!

@calebdw
Copy link
Contributor Author

calebdw commented Feb 7, 2024

Looks like tests are currently failing on 2.x for L11

@szepeviktor
Copy link
Collaborator

Yes, the scheduled workflow also fails
https://github.com/larastan/larastan/actions/runs/7807784230/job/21296839699

@canvural
Copy link
Collaborator

canvural commented Feb 7, 2024

Hi,

I think we can use config_path() function and do the comparison with that.

Also I do not remember exactly now, but we might need to check that the function is actually coming from Laravel. Or at least that the function is in global namespace and not in Some\Namespace\env

Lastly, I'd prefer that it is disabled by default.

@calebdw
Copy link
Contributor Author

calebdw commented Feb 8, 2024

Done

Copy link
Collaborator

@szepeviktor szepeviktor left a comment

Choose a reason for hiding this comment

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

Laravel 9 has no useConfigPath

@canvural
Copy link
Collaborator

canvural commented Feb 8, 2024

What's useConfigPath?

@szepeviktor
Copy link
Collaborator

A method called in this PR: tests/Rules/NoEnvCallsOutsideOfConfigRuleTest.php

@calebdw
Copy link
Contributor Author

calebdw commented Feb 8, 2024

@szepeviktor should be fixed now, thanks for catching!

@calebdw calebdw changed the title Detect usage of env() function outside from config folder feat: detect usage of env() function outside of config folder Feb 9, 2024
@canvural canvural merged commit 7b01d2e into larastan:2.x Feb 11, 2024
30 checks passed
@canvural
Copy link
Collaborator

Thank you!

@calebdw calebdw deleted the env branch April 17, 2024 00:35
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.

Detect usage of env() function outside from config folder
4 participants