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

[Bug]: Strict type checking falsely stating success #1007

Open
Tklaversma opened this issue Nov 14, 2023 · 3 comments · May be fixed by #1100
Open

[Bug]: Strict type checking falsely stating success #1007

Tklaversma opened this issue Nov 14, 2023 · 3 comments · May be fixed by #1100
Labels

Comments

@Tklaversma
Copy link

What Happened

The following test will check if declare(strict_types=1); is in any file within the directory app.

test('Application uses strict typing')
    ->expect('App')
    ->toUseStrictTypes();

However, it is not checking if the line is actually being used. For example, whenever you comment the line, the test will still return true.

I would expect to check if the line is used.

How to Reproduce

  1. Define the following test:
test('Application uses strict typing')
    ->expect('App')
    ->toUseStrictTypes();
  1. Comment declare(strict_types=1); in a file within the directory app.

  2. Run the test and see it succeed, instead of returning failed.

Sample Repository

No response

Pest Version

2.24.3

PHP Version

8.1

Operation System

macOS, Windows, Linux

Notes

No response

@Tklaversma Tklaversma added the bug label Nov 14, 2023
@dasundev
Copy link

Yeah, this is confirm. I have also noticed this issue right now.

@owenvoke
Copy link
Member

Yeah, this only uses a str_contains() on the file contents.

I think what would make the most sense is to probably have a regex check for something like:

^<\?php\s+declare\(.*?strict_types\s?=\s?1.*?\);

Preview on regex101

That would be slower, but would resolve the issue. 🤷🏻 It would also ensure that it's at the start of a line (so no // <?php declare(strict_types=1);) and also support a declare() usage that includes ticks or encoding.

@kusab85
Copy link

kusab85 commented Feb 21, 2024

Bumping up the issue. Minor thing but annoying.

@faissaloux faissaloux linked a pull request Feb 21, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants