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

docs: add correct array signatures for Modules #8106

Merged
merged 1 commit into from
Oct 28, 2023

Conversation

paulbalandan
Copy link
Member

Description
See #7731

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

@@ -72,7 +72,7 @@ class Modules extends BaseModules
*
* If it is not listed, only the base application elements will be used.
*
* @var string[]
* @var list<string>
Copy link
Member

Choose a reason for hiding this comment

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

If list is okay with @var, we can remove @phpstan-var tags like:

     * @var array<int, string>
     * @phpstan-var list<string>

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting. I think vscode intelephense has now support for this type, as well as Closure signatures.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, then let's remove @phpstan-var from now on.

Copy link
Member

Choose a reason for hiding this comment

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

I'm all for this; I can confirm that Intelephense (in BBEdit) supports list as well.

Copy link
Member

Choose a reason for hiding this comment

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

I know I always ask this... these are different, right?

list<string>
string[]

I think the latter just means "values are all strings" without any indication of keys. But I can't find the answer explicitly.

Copy link
Member

Choose a reason for hiding this comment

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

Yes.

/** @return ValueType[] */
In Psalm this annotation is equivalent to @psalm-return array<array-key, ValueType>.
https://psalm.dev/docs/annotating_code/type_syntax/array_types/#phpdoc-syntax

Copy link
Member Author

Choose a reason for hiding this comment

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

same for phpstan

Copy link
Member Author

Choose a reason for hiding this comment

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

Sent PR: #8114

@kenjis kenjis changed the title fix: add correct array signatures for Modules docs: add correct array signatures for Modules Oct 27, 2023
@kenjis
Copy link
Member

kenjis commented Oct 27, 2023

I changed fix: in the subject to docs: because when checking the changelog generated at release time, we need to think a bit whether the item is a bug fix or an item that should be removed from the changelog.
I would like to limit fix: to product code fixes only.

@MGatner
Copy link
Member

MGatner commented Oct 28, 2023

@paulbalandan Since it's vaguely related could you roll in the CS Fixer change to this PR?

+++ /home/runner/work/CodeIgniter4/CodeIgniter4/system/Exceptions/FrameworkException.php

@kenjis kenjis merged commit 243a6be into codeigniter4:develop Oct 28, 2023
61 checks passed
@paulbalandan paulbalandan deleted the phpstan-modules branch October 29, 2023 07:12
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