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

CRM_Utils_Type::validatePhpType - Helper to validate PHP type expressions #20923

Merged
merged 1 commit into from
Jul 25, 2021

Conversation

totten
Copy link
Member

@totten totten commented Jul 21, 2021

Overview

Adds a helper for evaluating the type expressions that appear in PHP docblocks. This is a building block for other code that uses docblocks for validation.

Technical Details

Here are a few examples:

CRM_Utils_Type::validatePhpType('abc', 'string'); // TRUE
CRM_Utils_Type::validatePhpType(123, 'string'); // FALSE
CRM_Utils_Type::validatePhpType('abc', 'string|int|\DateTime'); // TRUE
CRM_Utils_Type::validatePhpType(123, 'string|int|\DateTime'); // TRUE
CRM_Utils_Type::validatePhpType(new \DateTime(), 'string|int|\DateTime'); // TRUE
CRM_Utils_Type::validatePhpType(new \stdClass(), 'string|int|\DateTime'); // FALSE
CRM_Utils_Type::validatePhpType([10,20], 'int[]'); // TRUE
CRM_Utils_Type::validatePhpType([10,20], 'string[]'); // FALSE

The default mode of operation is strict. In strict usage, string "123" is not an instance of int, even though it looks like 123.

Optionally, you may use a non-strict mode. In this case, string "123" and int 123 are interchangeable. (Both "123" and 123 satisfy both type-constraints for string and int.) This may be more appropriate if you are exchanging data through softly typed interfaces (e.g. PHP-MySQL, PHP-XML, APIv{$n}).

@civibot
Copy link

civibot bot commented Jul 21, 2021

(Standard links)

@civibot civibot bot added the master label Jul 21, 2021
@seamuslee001
Copy link
Contributor

@totten how does this differ from the CRM_Utils_Rule:: stuff?

@totten
Copy link
Member Author

totten commented Jul 21, 2021

@seamuslee001 Well, I believe that CRM_Utils_Rule originates in providing a laundry-list of validators for QuickForm elements (eg numeric, color, email, positiveInteger, mysqlColumnNameOrAlias). These are almost always strings or numbers.

CRM_Utils_Type::validatePhpType() is a validator for a PHP type-expression like int[] or MyClass.

I suppose there's some overlap - both the QuickForm validator-system and the PHP type-systems have a generic string and generic integer. But beyond that, they seem to go in different directions.

To give some more context -- here's an excerpt from the current draft of WorkflowMessage modeling (example/test-case). The PHP classes have @var int, @var string[], etc. because all the other PHP tooling expects those.

      /**
       * @var int
       * @scope tpl as my_int
       */
      protected $myProtectedInt;

      /**
       * @var string[]
       */
      protected $implicitStringArray;

      public function validate(): array {
        /* inherited function; base implementation reads annotations and validates data */
      }

The validate() method looks at the field list and runs validators. So it'll read the @var and call CRM_Utils_Type::validatePhpType() to ensure that (1) $myProtectedInt is really int and (2) $implicitStringArray is really string[].

I suppose... if one wanted to incorporate the validators from CRM_Utils_Rule into a class-model, we could define a new annotation (maybe @rule or @validate). Like:

      /**
       * @var string
       * @rule color
       */
      protected $background;

      /**
       * @var string
       * @rule color
       */
      protected $foreground;

      /**
       * @var int
       * @rule positiveInteger
       */
      protected $widthCm;

The validate() method could then enforce both@var and @rule...

@seamuslee001
Copy link
Contributor

As mentioned on the CT call I am comfortable with this as concept just figured something might already be doing this, Coleman is going to review this as well

@eileenmcnaughton eileenmcnaughton added the merge ready PR will be merged after a few days if there are no objections label Jul 24, 2021
@eileenmcnaughton
Copy link
Contributor

I've given this merge-ready - if @colemanw doesn't come back with some apiv4 function we should be taking into account this should be merged

@colemanw
Copy link
Member

Yea I don't see anything in APIv4. This looks good.

@colemanw colemanw merged commit 4ca50ef into civicrm:master Jul 25, 2021
@totten totten deleted the master-phptype branch July 25, 2021 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-test master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants