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

Allow assertions for early exit strategies #32

Open
ScreamingDev opened this issue Jan 25, 2019 · 3 comments
Open

Allow assertions for early exit strategies #32

ScreamingDev opened this issue Jan 25, 2019 · 3 comments

Comments

@ScreamingDev
Copy link

This would be reported:

<?php

if ( 'cli' !== PHP_SAPI ) {  // THIS LINE
  throw new \Exception('only from CLI');
}

echo 'hi';

We check if the current system state is something that we can work with. In general we reject everything except the one working thing which is an assertion IMHO.

This wouldn't be reported:


<?php

if ( 'cli' === PHP_SAPI ) {
  echo 'hi';
} else {
  throw new \Exception('CLI!');
}

Which makes the code unreadable if you think of 4-5 paths which are then nested damn deep.
Also PHPMD may nag about the "else" and the linux kernel standards says

Now, some people will claim that having 8-character indentations makes the code move too far to the right, and makes it hard to read on a 80-character terminal screen. The answer to that is that if you need more than 3 levels of indentation, you’re screwed anyway, and should fix your program.

So please tell me how we can avoid the first expression to be marked as an "assumption".

@rskuipers
Copy link
Owner

There's currently no way to exclude this. This tool is also not in active development. But feel free to create a PR with a proposal and we can look into implementing it.

I would also argue that it shouldn't be a goal to have php-assumptions report 0 warnings. There are always edge-cases.

@ScreamingDev
Copy link
Author

Thanks for your answer.

Before I do a fork I need to ask: Has there been a specific reason or problem with feasibility why this is not based on PHPCS?

Just curious because having a configurable way (so using the PHPCS xml) for all those edges would be great!

@rskuipers
Copy link
Owner

@ScreamingDev I'm not too sure whether I considered that. I may have gone down this path to learn more about how to write a static analyser. If you can somehow turn it into a rule that would be awesome of course.

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

2 participants