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

refactor: Fix phpstan codeigniter.frameworkExceptionInstance #9389

Conversation

neznaika0
Copy link
Contributor

Description
Invoking an FrameworkException using the method only

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@neznaika0
Copy link
Contributor Author

Can you tell me why my GA is crashing? It's a copy of this repository.
https://github.com/neznaika0/codeigniter-dev/actions/runs/12687268125/job/35361295486
I tried adding --with-all-dependencies and it installed all the packages.

@samsonasik
Copy link
Member

you probably need to mirror your fork repo with this repo tag as well, as it seems you don't have any tag at https://github.com/neznaika0/codeigniter-dev/tags , if the PR is green, i think it is fine just as is, as it may related with branch alias

"extra": {
"branch-alias": {
"dev-develop": "4.x-dev",
"dev-master": "4.x-dev"
}
},

/cc @paulbalandan

@neznaika0
Copy link
Contributor Author

I added all tags and master branch. It didn't help

@paulbalandan
Copy link
Member

My best guess is that neznaika0/framework is not the same as codeigniter4/framework.

I think this can be "fixed" by not running the GA in the forks.

@paulbalandan paulbalandan added the testing Pull requests that changes tests only label Jan 9, 2025
@neznaika0 neznaika0 force-pushed the refactor/phpstan-framework-exception-instance branch from 07205a1 to 34a41d3 Compare January 9, 2025 11:36
@neznaika0
Copy link
Contributor Author

There seems to be a mistake in applying the rules comment_to_phpdoc

@paulbalandan
Copy link
Member

If you have time, pls send a PR to the coding-standard repo adding @phpstan-ignore in the ignored tags of comment_to_phpdoc. I'll gladly merge it.

@neznaika0
Copy link
Contributor Author

But before that, he ignores @phpstan-ignore Only now it showed up

@paulbalandan
Copy link
Member

I looked at the code of comment_to_phpdoc. If there's a variable assignment it treats it as a structural element so it changes the comment to a phpdoc.

@neznaika0
Copy link
Contributor Author

Rewrite as phpDoc?

@paulbalandan
Copy link
Member

Yes. This is what it looks like:

This is correct because the @var needs to be in a phpdoc so that IDEs can understand that $foo is an int.

-// @var int $foo
+/** @var int $foo */
 $foo = some_function();

Continuing from that, if the comment is the last in the line, it will be considered for the next line. So, the fixer thinks the comment is for the assignment to $e2 and changes the comment to a phpdoc.

@neznaika0 neznaika0 force-pushed the refactor/phpstan-framework-exception-instance branch from 34a41d3 to df17203 Compare January 9, 2025 15:39
Copy link

👋 Hi, @neznaika0!

We detected conflicts in your PR against the base branch 🙊
You may want to sync 🔄 your branch with upstream!

Ref: Syncing Your Branch

@github-actions github-actions bot added the stale Pull requests with conflicts label Jan 13, 2025
@neznaika0 neznaika0 force-pushed the refactor/phpstan-framework-exception-instance branch from df17203 to a716b21 Compare January 13, 2025 04:13
@neznaika0 neznaika0 force-pushed the refactor/phpstan-framework-exception-instance branch from a716b21 to db90e5f Compare January 13, 2025 04:16
@paulbalandan paulbalandan removed the stale Pull requests with conflicts label Jan 13, 2025
@paulbalandan paulbalandan merged commit a3704a1 into codeigniter4:develop Jan 13, 2025
39 checks passed
@neznaika0 neznaika0 deleted the refactor/phpstan-framework-exception-instance branch January 13, 2025 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Pull requests that changes tests only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants