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

PHPStan Ignore File #4065

Merged
merged 3 commits into from
Jan 5, 2021
Merged

PHPStan Ignore File #4065

merged 3 commits into from
Jan 5, 2021

Conversation

MGatner
Copy link
Member

@MGatner MGatner commented Jan 5, 2021

Description
We’ve run into our first PHP 7/8 PHPStan issue: https://github.com/codeigniter4/CodeIgniter4/runs/1650630303

For whatever reason this line in CITestStreamFilter is no longer an issue in PHP 8, but persists in PHP 7:

	public function filter($in, $out, &$consumed, $closing)
	{
		while ($bucket = stream_bucket_make_writeable($in))
		{
			static::$buffer .= $bucket->data;

			$consumed += $bucket->datalen;
		}

		// @phpstan-ignore-next-line
		return PSFS_PASS_ON;
	}

Ondre is considering work on how to improve this split handling (phpstan/phpstan#4223 by our own @paulbalandan, phpstan/phpstan#4060 (comment)) but the current solution is to have different config files for each version.

Given that the problem currently is just single, small, test file I propose we ignore it for all versions and postpone dealing with this issue until we see what Ondre does upstream.

Checklist:

  • Securely signed commits
  • Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • n/a User guide updated
  • Conforms to style guide

@samsonasik
Copy link
Member

The // @phpstan-ignore-next-line can be removed in CITestStreamFilter.php

Copy link
Member

@paulbalandan paulbalandan left a comment

Choose a reason for hiding this comment

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

I guess this is our best resort for now pending Ondrej's decision. I would like to have the // @phpstan-ignore-next-line removed to be a bit cleaner since we are now ignoring this file altogether.

@MGatner
Copy link
Member Author

MGatner commented Jan 5, 2021

Oh right! Thanks. Removed!

@MGatner MGatner merged commit 5a8e757 into codeigniter4:develop Jan 5, 2021
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.

3 participants