Skip to content

Collect parameter attributes #37

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

Merged
merged 8 commits into from
Jun 17, 2025
Merged

Collect parameter attributes #37

merged 8 commits into from
Jun 17, 2025

Conversation

olvlvl
Copy link
Owner

@olvlvl olvlvl commented Jun 15, 2025

Original PR: #33

Testing

I run the following script on a fresh Symfony app, before the commit to ignore SensitiveParameter:

<?php

use olvlvl\ComposerAttributeCollector\Attributes;

require_once './vendor/autoload.php';
require_once './vendor/attributes.php';

$targets = Attributes::findTargetParameters(SensitiveParameter::class);

var_dump($targets);

And got the following output (excerpt):

array(184) {
  [0]=>
  object(olvlvl\ComposerAttributeCollector\TargetParameter)#7 (4) {
    ["attribute"]=>
    object(SensitiveParameter)#6 (0) {
    }
    ["class"]=>
    string(29) "Doctrine\DBAL\Tools\DsnParser"
    ["method"]=>
    string(5) "parse"
    ["name"]=>
    string(3) "dsn"
  }

@coveralls
Copy link

coveralls commented Jun 15, 2025

Pull Request Test Coverage Report for Build 15702633794

Details

  • 82 of 88 (93.18%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 91.202%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/ClassAttributeCollector.php 40 41 97.56%
src/Collection.php 18 23 78.26%
Totals Coverage Status
Change from base Build 15702569366: 0.1%
Covered Lines: 425
Relevant Lines: 466

💛 - Coveralls

@olvlvl olvlvl added the enhancement New feature or request label Jun 16, 2025
@olvlvl olvlvl added this to the v2.1.0 milestone Jun 16, 2025
@olvlvl olvlvl self-assigned this Jun 16, 2025
@staabm
Copy link
Contributor

staabm commented Jun 16, 2025

thanks for taking over and looking into it.

I tried in phpstan-src but it did not work for me.
I guess what is missing is this commit ondrejmirtes@48124ab which was a fix which landed after my initial PR and was not part of #33

@olvlvl
Copy link
Owner Author

olvlvl commented Jun 16, 2025

@staabm I'm not sure what's happening in the commit you mentioned. Could you explain how you test it and what I should look for?

@staabm
Copy link
Contributor

staabm commented Jun 16, 2025

what I tried is here: phpstan/phpstan-src#4064

repro

  • checkout pull request
  • composer install
  • make test

errors with

➜  phpstan-src git:(test-olvlvl) ✗ make tests
composer install --working-dir tests
Xdebug: [Step Debug] Could not connect to debugging client. Tried: localhost:9003 (through xdebug.client_host/xdebug.client_port).
Installing dependencies from lock file (including require-dev)
Verifying lock file contents can be installed on current platform.
Nothing to install, update or remove
Generating autoload files
36 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
XDEBUG_MODE=off php tests/vendor/bin/paratest --runner WrapperRunner --no-coverage
ParaTest v7.8.3 upon PHPUnit 11.5.23 by Sebastian Bergmann and contributors.


In Resolver.php line 649:
                                                                                                                                                                                                                                                  
  Service of type PHPStan\Analyser\NodeScopeResolver: Service of type PHPStan\Reflection\SignatureMap\SignatureMapProvider required by $signatureMapProvider in NodeScopeResolver::__construct() not found. Did you add it to configuration file  
  ?                                                                                                                                                                                                                                               
                                                                                                                                                                                                                                                  

paratest [--functional] [-m|--max-batch-size MAX-BATCH-SIZE] [--no-test-tokens] [--passthru-php PASSTHRU-PHP] [-p|--processes PROCESSES] [--runner RUNNER] [--tmp-dir TMP-DIR] [-v|--verbose] [--bootstrap BOOTSTRAP] [-c|--configuration CONFIGURATION] [--no-configuration] [--cache-directory CACHE-DIRECTORY] [--testsuite TESTSUITE] [--exclude-testsuite EXCLUDE-TESTSUITE] [--group GROUP] [--exclude-group EXCLUDE-GROUP] [--filter FILTER] [--process-isolation] [--strict-coverage] [--strict-global-state] [--disallow-test-output] [--dont-report-useless-tests] [--stop-on-defect] [--stop-on-error] [--stop-on-failure] [--stop-on-warning] [--stop-on-risky] [--stop-on-skipped] [--stop-on-incomplete] [--fail-on-incomplete] [--fail-on-risky] [--fail-on-skipped] [--fail-on-warning] [--fail-on-deprecation] [--order-by ORDER-BY] [--random-order-seed RANDOM-ORDER-SEED] [--colors [COLORS]] [--no-progress] [--display-incomplete] [--display-skipped] [--display-deprecations] [--display-errors] [--display-notices] [--display-warnings] [--teamcity] [--testdox] [--log-junit LOG-JUNIT] [--log-teamcity LOG-TEAMCITY] [--coverage-clover COVERAGE-CLOVER] [--coverage-cobertura COVERAGE-COBERTURA] [--coverage-crap4j COVERAGE-CRAP4J] [--coverage-html COVERAGE-HTML] [--coverage-php COVERAGE-PHP] [--coverage-text [COVERAGE-TEXT]] [--coverage-xml COVERAGE-XML] [--coverage-filter COVERAGE-FILTER] [--no-coverage] [--] [<path>]

make: *** [tests] Error 1

maybe @ondrejmirtes can give us a idea what ondrejmirtes@48124ab was about and whether its related (or my test was wonky)

@olvlvl olvlvl merged commit 29749d4 into main Jun 17, 2025
13 checks passed
@olvlvl olvlvl deleted the method-params branch June 17, 2025 09:19
@staabm
Copy link
Contributor

staabm commented Jun 17, 2025

thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants