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

Meaning of integer' and float' #69

Closed
ruafozy opened this issue Feb 19, 2015 · 19 comments
Closed

Meaning of integer' and float' #69

ruafozy opened this issue Feb 19, 2015 · 19 comments

Comments

@ruafozy
Copy link

ruafozy commented Feb 19, 2015

The PHPDoc draft fails to make it clear whether the types integer and
float are to be interpreted "semantically" or are defined in terms of
their corresponding PHP type-test functions (in this case, is_integer and
is_float). I will illustrate the problem with the following code.

    /**
     * @param int $n
     */
    function double($n) {
      return 2 * $n;
    }

    $x = 0;

    $x += double(1);     // call 1
    $x += double(1.0);   // call 2
    $x += double("1");   // call 3

With reference to that code:

  1. Which of the function calls are correct?
  2. If some are incorrect, what is the simplest PHPDoc type for $n which
    makes all the calls correct?
@index0h
Copy link

index0h commented Jun 29, 2015

@ruafozy You created func which not apply it's own interface, declared in php doc. You must check input arguments at least for the type.

/**
 * @param int $n
 * @return int
 * @throws \InvalidArgumentExeption
 */
function double($n) {
    if (!is_int($n)) {
        throw new \InvalidArgumentExeption('Param $n must be int');
    }

    return 2 * $n;
}

Correct call is only $x += double(1); // call 1.
2 and 3 must throw exception.

@loren-osborn
Copy link

There is an issue with this interpretation, that I believe @ruafozy was trying to point out, because of PHP's notorious type coercion behavior.

First I think we all agree that these calls should throw exceptions (if the function is implemented to enforce it's contract, as @index0h suggested):

    $x += double(1.5);     // call 1
    $x += double("1.5");   // call 2
    $x += double("abc");   // call 3

As @index0h suggests, we certainly could require the function argument be an actual integer type (with is_integer() for instance), but I believe, (and I believe @ruafozy was suggesting) that this literal interpretation obscures the semantic intent of @param int $n that is more in keeping with PHP's general design philosophy. In PHP the caller is generally not expected to care how scalar values are stored in a variable, as long as the data represents an intended legal value.

Look, for instance, at this alternate semantic interpretation of @param int $n:

/**
 * @param int $n
 * @return int
 * @throws \InvalidArgumentExeption
 */
function double($n) {
    if (!preg_match('/^-?\\d+$/', "{$n}")) {
        throw new \InvalidArgumentExeption(
            sprintf('Param $n (%s) must be int', var_export($n, true)));
    }

    return 2 * intval($n, 10);
}

I'm not saying that either interpretation is "correct", but I do believe that the semantic interpretation is more PHP like.

What do you think?

@GrahamCampbell
Copy link

The PHPDoc draft fails to make it clear whether the types integer and float

There is no such type as integer. PHP 7 defines the correct types as string, int, float, bool. Anything else is just an alias.

@loren-osborn
Copy link

@GrahamCampbell, if we are discussing literal types, http://php.net/manual/en/language.types.integer.php refers to them pretty pervasively as "integers", but I believe the question is, are we (or should we be) talking about literal types or semantic types. We could, for instance support both literal integers, and semantic integers as distinct types, which would require distinct names. I am rather confident that distinguishing between literal and semantic integers as integer vs. int would be very confusing, and should be avoided.

To clarify, I consider a literal integer to be a value stored as an integer, while a semantic integer would be an integer, a float or a string that represents a number with no fractional part.

@Fleshgrinder
Copy link

Weak type checks as defined for PHP7: https://wiki.php.net/rfc/scalar_type_hints_v5 The RFC includes a table that explains what is accepted and what is not (this also applies to previous PHP versions).

@ruafozy
Copy link
Author

ruafozy commented Aug 1, 2015

@loren-osborn,

Above you wrote that you "believe that the semantic
interpretation is more PHP like". I agree. For
example, consider the following code:

  $total = array_sum(file('php://stdin'));

This is perfectly good PHP code, and it shows that it
can be valid for the programmer to think of values as
numbers even though PHP considers them to be strings.

Because of PHP's type coercion, I believe that PHPdoc
would be going against the spirit of PHP if it
specified that int can denote only those values for
which is_integer returns true. In other words, I
think PHPDoc's int should denote a mathematical
integer, not a machine integer. Therefore, in the code
example in the first post in this thread, I think that
all 3 calls to double should be considered to be
consistent with the PHPDoc comment.

@GrahamCampbell,

the PHPDoc type is called int, not integer; you are
therefore correct to point out that I made a mistake in
writing integer.

@GrahamCampbell
Copy link

No, the type in php is called INT.

@loren-osborn
Copy link

@ruafozy, I see room in the spec for supporting both type hints. int or integer could refer, for instance, to a semantic integer, where strict int or strict integer could refer to a strict literal integer data type (where strict could act as a type decorator). What the actual type distinctions should be, should be open for discussion and consensus.

@GrahamCampbell, int very well may be the official name for a PHP integer. I don't think it matters very much, as integer is an alias for int in most relevant cases. I think the ability to distinguish between a literal and semantic type hint is a much more valuable discussion.

I welcome your feedback.

@mvriel
Copy link
Member

mvriel commented Aug 2, 2015

As I see it, the types are to be interpreted in a semantical way as it is a means to communicate with a reader what sort of information is expected. I consider the enforcing of type correctness, or validating whether the contents are truly of that sort, something that should be solved in code and not in PHPDoc.

I don't think it adds much for the reader's awareness whether you are talking about a literal integer or not. I think it should be a convention in a project how literal you interpret the documentation.

@Fleshgrinder
Copy link

I don't think it matters very much, as integer is an alias for int in most relevant cases.

It is an alias and that is why both keywords have to have the same meaning in PHPDoc as they have in PHP: they refer to semantically to an integer number.

I think it should be a convention in a project how literal you interpret the documentation.

Exactly and in the future declare(strict_types=1) solves the problem for inter project usage.

@loren-osborn
Copy link

@mvriel, I do think there are (rare) times when it does make sense to specify a literal storage type, like int. It might be helpful with internal functions that perform conversions, for instance. I am aware that in PHP, this is typically an edge case, but I see some occasional value is exact type specification.

@Fleshgrinder, as these situations will probably be quite rare, I see no value in adding a project or file-wide setting. Can you provide an example where this would be useful? (If you're wanting project-wide type strictness, I suspect you're using the wrong language.)

@mvriel
Copy link
Member

mvriel commented Aug 4, 2015

@loren-osborn Since the current wording does not require you to interpret it literal my thoughts are that if a specific project needs to interpret the types literally (because they added business logic in their project that needs that to be) then they should have a Documentation Standard or Coding Standard that extends PSR-1/2 and PSR-5 that states so.

There is nothing in PSR-5 that specifies this is not allowed and to add specific rules for such rare occurrences that are not documentation related adds, in my opinion, unnecessary complexity.

@Fleshgrinder
Copy link

@loren-osborn and @mvriel the declare(strict_types=1) is a new feature of PHP7.

@index0h
Copy link

index0h commented Aug 5, 2015

@loren-osborn

/**
 * @param int $n <- Here you said that argument MUST be int, not should, not may, but must.
 * @return int
 * @throws \InvalidArgumentExeption
 */
function double($n) {
    if (!preg_match('/^-?\\d+$/', "{$n}")) { // <- Here you cast argument to string, it's wrong flow.
        throw new \InvalidArgumentExeption(
            sprintf('Param $n (%s) must be int', var_export($n, true)));
    }

    return 2 * intval($n, 10);
}

What will be if $n - Exception instance? So you must check only if argument is int, in any other cases developer uses func incorrectly. This situation could not be resolved automatically so you thrown an exception.

@loren-osborn
Copy link

@index0h, I regret I'm having a little trouble with your English, but I will try my best to answer. I apologize if this is long-winded. 😦 I see you bringing up 3 distinct issues:

Role of the caller argument specification

The first issue is the role of the caller argument specification @param int $n. I need to clarify a bit what you might mean by "argument MUST be int". I will assume:

The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
"SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL"
in this document are to be interpreted as described in RFC 2119.

RFC 2119 specifies that MUST means:

that the definition is an absolute requirement of the specification.

... so if we look, for instance, at RFC 2045 (the MIME specification) which says compliant messages "MUST include" the "verbatim text" MIME-Version: 1.0, this does not mean that all messages, everywhere need to include this string. It means if they don't include this string they aren't compliant with the specification (and are therefore not MIME encoded messages).

So, if we return to @param int $n in the function comment, that is simply a spec that the caller is expected to conform to. The question is, what happens when that spec is violated? In some strongly typed, compiled languages, this can easily be handled with a compilation failure. In PHP we don't have that option. With classes or interfaces we can use type hinting. (From PHP.net: Type Hinting) "Failing to satisfy the type hint results in a catchable fatal error." This demonstrates two things:

  1. Not only is it possible for the caller to violate the spec, the situation has a specific, defined behavior.
  2. Violating the spec has a fatal (or fatal-ish) behavior, because it indicates that something is very wrong.
    This is exactly what exceptions were designed for: indicating that something is very wrong and the program doesn't know how to proceed (unless the exception is specifically handled.) In PHP, exceptions are generally less cumbersome than the "catchable fatal error"s used in the built-in type hinting mechanism.

The meaning of int in the @param int $n comment

The second issue you brought up was the meaning of int in the caller argument specification. This is basically documenting the intended type to compensate for a limitation of the PHP type-hinting mechanism. int in the rest of PHP generally specifies a value stored internally as a signed integer number, not just a value that has the semantic meaning of an integer number (for instance a double with no fractional part, or a string representing a number.)

While I agree this might be inconsistent with the usage of int in the rest of PHP, the subject of much of this thread was if the inconsistent usage of int to indicate a semantic integer, rather than a strictly typed int, would be more apropriate for PHPDoc type hints. As such, this was an example of how the alternate meaning of int might work in this context, rather than a correct usage based on its current meaning.

My type validation check

The final issue you brought up was my type validation check. My code here was nothing to be proud of. It was also pretty inefficient. I intended it to illustrate that (in this example) an integer, double or string representation of an integer would all be considered valid. I might rewrite this, for instance, as:

    if (!is_int($n)) {
        $trimmedN = trim("{$n}");
        if (!is_numeric($trimmedN) || (abs($trimmedN - round($trimmedN)) > 0.001)) {
            throw new \InvalidArgumentExeption(
                sprintf('Param $n (%s) must be an integer', var_export($n, true)));
        } else {
            $n = intval(round($trimmedN), 10);
        }
    }

I'm not saying that this is much better, but it might be slightly more efficient if the input already is in integer format, It's likely a bit easier to read, and it ignores enclosing whitespace, and floating point rounding errors. That said, it's also big and cumbersome for a single argument validation. My point was to show off what could be considered a valid "semantic int" in contrast to a strict literal int, as this seemed more fitting with how PHP typically behaves.

@Fleshgrinder
Copy link

Just found this https://github.com/eloquent/typhax where he's using string for a literal string and stringable for other stuff that can easily be casted to a string. All in all, it contains many interesting type hint ideas.

@index0h
Copy link

index0h commented Aug 24, 2015

@loren-osborn sorry for my English((

"semantic int" in your case means "anything that php can cast to int". It means that @return int could return anything that could be cast as int, it could be string for example. In your case it'l be correct. But it's way to nowhere.

/**
 * @param int $n
 * @return int
 * @throws \InvalidArgumentException
 */
function double($n) {
    if (!is_int($n)) {
        $trimmedN = trim("{$n}");
        if (!is_numeric($trimmedN) || (abs($trimmedN - round($trimmedN)) > 0.001)) {
            throw new \InvalidArgumentException(
                sprintf('Param $n (%s) must be an integer', var_export($n, true)));
        } else {
            $n = intval(round($trimmedN), 10);
        }
    }

    return strval(2 * $n);
}

This example is disgusting, but it return semantic int.

So in most part of methods I dont know what exactly they will work with and must cast every argument. In this case erros count will be grow.

I offer to use strong type declaration. If some where in code my func doubleInt called with float or string - it means: my code aroud it is incorrect, it not applies doubleInt interface. But with semantic int I dont even know about the problem.

Look, if you admit extra types in your function, like float or string - you should type it in dockblock @param int|float|string. It's ok.

@Fleshgrinder
Copy link

Although I was pro weak types before, I would like to state that I changed my mind. I think that it is easier if the given type is to be interpreted in a strict manner. It is easier for the reader if a string is a string and an integer is an integer.

I think it would be best to apply the PHP 7 rules for strict types to the types in the PHPDoc. This means that every type is absolutely strict with one exception: an integer without a fraction part is a valid float.

@mvriel
Copy link
Member

mvriel commented Sep 6, 2015

Types in this PSR are intended to be semantic. This means that, for example, if a piece of code mentions an int then in reality it could be an integer, float without numeric part or strong containing a valid number.

Applying strictness and guarding for such strictness is not the responsibility of the PHPDoc standard or DocBlocks in general.

A project may decide to interpret a type more strict than is intended by this PSR but this is a project specific documentation standard item and should not be blocked by this PSR. This information should be clearly mentioned in the PSR and I will as this to my review list

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

No branches or pull requests

6 participants