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

lncorrect PHPDoc tags for stubs (e.g. acf_get_field_types, acf_get_setting) #9

Open
mundschenk-at opened this issue Dec 25, 2022 · 10 comments
Labels
help wanted Extra attention is needed

Comments

@mundschenk-at
Copy link

mundschenk-at commented Dec 25, 2022

Unfortunately the included PHPDoc tags for several functions are incorrect, for example @return void for acf_get_field_types and acf_get_setting).

@mundschenk-at mundschenk-at changed the title lIncorrect lncorrect PHPDoc tags for stubs (e.g. acf_get_field_types, acf_get_setting) Dec 25, 2022
@szepeviktor
Copy link
Member

szepeviktor commented Dec 25, 2022

Yes. This is not a selling point of ACF Pro, so these tags are slowly improving.
Please talk to Elliot as this is an automatically generated repo.

All I have is this.

# Remove determine_locale function
sed -i -e 's#^function determine_locale()$#//&#' "$FILE"
# - Make docblocks start with "/**"
# - Fix type and variable name order for @param
# - Remove remaining parentheses for @param
# - Fix type and variable name order for @return
# - Remove remaining parentheses for @return
# - Fix "void"
find . -type f -name "$FILE" -exec sed \
-e 's#^\(\s*/\*\)$#\1*#' \
-e 's#^\(\s*\*\s*@param\s\+\)\(\$\S\+\)\s\+(\(\S\+\))\(.*\)$#\1\3 \2\4#' \
-e 's#^\(\s*\*\s*@param\s\+\)(\(\S\+\))\(.*\)$#\1\2\3#' \
-e 's#^\(\s*\*\s*@return\s\+\)\(\$\S\+\)\s\+(\(\S\+\))\(.*\)$#\1\3 \2\4#' \
-e 's#^\(\s*\*\s*@return\s\+\)(\(\S\+\))\(.*\)$#\1\2\3#' \
-e 's#n/a#void#i' \
-i "{}" ";"

Related #8 and many more.

What do you suggest?

@mundschenk-at
Copy link
Author

I don't really have a suggestion, I just noted that paulthewalton/acf-stubs was deprecated and it was advised to install php-stubs/acf-pro-stubs instead, but doing so resulted in invalid findings with PHPStan.

@szepeviktor
Copy link
Member

szepeviktor commented Dec 25, 2022

Yes, Paul did it with his hands, line by line.

I would be very happy if you figure out a mechanism to fix the docblock of daily used functions.

@szepeviktor
Copy link
Member

Yes, Paul did it with his hands, line by line.

No! My mistake. Paul also used giacocorsiglia/stubs-generator 3 years ago.

@szepeviktor
Copy link
Member

...after a bit of thinking. Please try to talk to Elliot. Maybe he accepts a patch.

Last time I talked to him 🤣 he told me that applying my patch would fix all docblocks and he prefers fixing source code and docblocks at the same time, so he did not apply it.

@szepeviktor
Copy link
Member

szepeviktor commented Mar 12, 2023

@mundschenk-at All right. Please provide me that one file (includes/fields.php) with fixed types in a secret gist and I figure out something.

@szepeviktor
Copy link
Member

szepeviktor commented Jul 21, 2023

@mundschenk-at Did a series of releases but still no improvement on "your" front.

Many docblocks have improved in v6.0.7...v6.1.0 but not in these reported functions.

@szepeviktor szepeviktor added the help wanted Extra attention is needed label Jul 21, 2023
@crstauf
Copy link

crstauf commented Sep 7, 2023

Using acf()->fields->get_field_type() was recommended (#8 (comment)), but that throws an undefined property error.

@szepeviktor
Copy link
Member

You could download https://patch-diff.githubusercontent.com/raw/php-stubs/acf-pro-stubs/pull/8.patch and apply it as a patch.

@crstauf
Copy link

crstauf commented Sep 8, 2023

Ended up using composer-patches:~1.0 to apply a patch (on composer install/update) to fix the return type of acf_get_field_type().

Generated the patch with this command:

git diff --no-index vendor/php-stubs/acf-pro-stubs/acf-pro-stubs.php.old vendor/php-stubs/acf-pro-stubs/acf-pro-stubs.php > dev/patches/acf-pro-stubs.patch

I tried using symplify/vendor-patches, but couldn't get the patch to apply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants