Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

Improve tests, asserions, CI setting and etc #312

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

peter279k
Copy link

@peter279k peter279k commented Nov 21, 2020

Changed log

  • Fix Psalm analysis error
  • Removing some comment annotations because they're unused.
  • Adding the pull_request trigger to let upcoming PR can do GitHub Action work automatically.
  • Fixing the AnnotationTest class namespace.
  • Using the assertCount to assert expected count is same as result.
  • To fix following Composer error, using the php-7.2 version to update cached dependencies:
Your lock file does not contain a compatible set of packages. Please run composer update.

  Problem 1
    - ocramius/package-versions is locked to version 1.4.2 and an update of this package was not requested.
    - ocramius/package-versions 1.4.2 requires composer-plugin-api ^1.0.0 -> found composer-plugin-api[2.0.0] but it does not match the constraint.
  Problem 2
    - ocramius/package-versions 1.4.2 requires composer-plugin-api ^1.0.0 -> found composer-plugin-api[2.0.0] but it does not match the constraint.
    - vimeo/psalm 3.12.1 requires ocramius/package-versions ^1.2 -> satisfiable by ocramius/package-versions[1.4.2].
    - vimeo/psalm is locked to version 3.12.1 and an update of this package was not requested.

ocramius/package-versions only provides support for Composer 2 in 1.8+, which requires PHP 7.4.
If you can not upgrade PHP you can require composer/package-versions-deprecated to resolve this with PHP 7.0+.

@@ -27,7 +27,7 @@ public function getCompiled(): array
*
* @param bool|string $data
* @param string $type
* @return bool|string
* @return bool|int|float|string
Copy link
Author

@peter279k peter279k Nov 21, 2020

Choose a reason for hiding this comment

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

To fix following Psalm error, using the @return bool|int|float|string annotation:

ERROR: InvalidReturnType - src/Compiler/Specification.php:30:16 - The declared return type 'bool|float|string' for Mill\Compiler\Specification::convertSampleDataToCompatibleDataType is incorrect, got 'scalar' (see https://psalm.dev/011)
     * @return bool|float|string

@@ -66,7 +66,7 @@ abstract class Annotation implements Arrayable
* @param null|string $method
* @param null|Version $version
*/
public function __construct(
final public function __construct(
Copy link
Author

Choose a reason for hiding this comment

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

To fix following Psalm error:

ERROR: UnsafeInstantiation - tests/Parser/Annotations/AnnotationTest.php:32:18 - Cannot safely instantiate class Mill\Parser\Annotation with "new $class_name" as its constructor might change in child classes (see https://psalm.dev/229)
                (new $annotation($this->getApplication(), $content, __CLASS__, __METHOD__))->process();


Copy link
Author

Choose a reason for hiding this comment

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

BTW, it can also use the @psalm-consistent-constructor annotation for the abstract Annotation class.

And it's up to the repository owner.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant