MDLSITE-4692 - It expects a PhpDoc for a class if you use a_class_name::class

[roperto]

My file at line 86:

$this->set_expected_exception(coding_exception::class);

Moodlecheck Output:

auth/outage/tests/phpunit/local/controllers/infopage_test.php
    Line 86: Class ) is not documented

[mudrd8mz]

Not saying you are not right. But can you please elaborate on why you actually want to use $this->set_expected_exception(coding_exception::class); instead of simpler $this->set_expected_exception('coding_exception'); I am just wondering.

[mudrd8mz]

It sounds a bit like "Hey John, what is your first name?" situation.

[roperto]

Hi David.

I just had this conversation with Brendan (I am assuming you know him) and the issue he raised is that ::class was implemented in PHP 5.5 and Moodle 27 (no longer supported cof cof) supports PHP 5.4.

IMHO it is good to use the ::class as it will help a lot your IDE to understand what is going on there. If you are refactoring things this REALLY come handy. Added benefit that if you are using imports you don't have to worry about the canonical name, it will resolve for you. If namespaces are long and you imported the class, jediform::class will resolve to \local_plugin\form\alliance\jediform which makes the ::class simpler.

All that said, I am new to Moodle development and I am open to adapt my coding practices to this platform. If you believe it is better to keep it as strings I will change my code in the plugin to make it consistent, no worries! :-)

Cheers,

Daniel

[mudrd8mz]

you don't have to worry about the canonical name, it will resolve for you

I must be missing something. How can this be used for importing the class from a namespace? If you wanted to use jediform::class in the use statement, you would already need to know what jediform is, no? Can you point me to a particular code using this?

[andrewnicols]

@mudrd8mz, one of the big benefits of supports the ::class syntax is that you do not have to escape namespaced names all the time. It is also really handy for things like mocks:

$mock = $this->getMockBuilder(\mod_forum\subscriptions::class);

versus.

$mock = $this->getMockBuilder("\\mod_forum\\subscriptions");

You can also use use:

use \mod_forum\subscriptions;

// ....
$mock = $this->getMockBuilder(subscriptions::class);

In the example that @roperto gives:

use \local_plugin\form\alliance\jediform;

// ...
$this->setExpectedException(jediform::class);

instead of

$this->setExpectedException("\\local_plugin\\form\\alliance\\jediform");

The above are particularly helpful when you have many instances of the name (multiple times that the exception is caught, or the mock is created).

It's also useful when creating things like your db/services.php, db/tasks.php, and friends as you can use the ::class syntax for the classname.

[roperto]

Hi @mudrd8mz ,

I think @andrewnicols gave the example I missed. When I said importing I meant using, my bad.

Here is one example of where I used it, but that just one of many places: https://github.com/catalyst/moodle-auth_outage/blob/issue36-pluginerrors/tests/phpunit/local/cli/cli_testcase.php#L86

Note I can move my cli_exception around, rename it, etc. without worrying about references (refactoring PHP code is not always easy).

Cheers, Daniel

[mudrd8mz]

Thanks guys.

you do not have to escape namespaced names all the time

If you did not use double quotes, you would not need to escape right? So this should just work:

$mock = $this->getMockBuilder('\mod_forum\subscriptions');

I do now see how this can be useful in unit tests as I did not realise that

$this->setExpectedException('Anakin');

would not work if the Anakin exception class was imported from another namespace. The method needs the class FDN here.

[roperto]

I am used to always escape backslashes anyway to follow the manual, but I agree that it is not needed.

According to the docs, to make a ' we escape with \' and to make a \ we escape with a \\.

But AFAIK if you use '\mod_forum\subscriptions' and alike you will get the expected result because you will not fall in the case of more than one backslash in sequence anyway, if PHP changes and adds a new special combination many things will break so I doubt they will.

http://php.net/manual/en/language.types.string.php#language.types.string.syntax.single

[brendanheywood]

While reducing the escaping and quoting is a nice side effect, I think it's missing the more important point. Using an opaque string is meaningless to a compiler and means you can't easily do static analysis on the code. Using the ::class adds meaning, so for instance if you had a typo then this would be a parser or compiler error rather than a runtime error. And as @roperto said you can also get the extra magic in IDE's like easier refactoring.

This is clearly a better way of doing things and we should not be penalized for it. But going further I'd argue it should even be the preferred way of doing things for all new code.

[roperto]

Hi @brendanheywood . I agree, but there is more: we can use the qualifiers self, static or parent. You could argue that we can find out self and parent and use the string instead to reference it, but I don't think the static can be resolved at compile (parse) time.

Example:

<?php
class A {
    public function foo() {
        echo "A::foo() called.\n";
        echo "  static=".(static::class)."\n";
        echo "    self=".(self::class)."\n";
    }
}

class B extends A {
    public function foo() {
        echo "B::foo() called.\n";
        echo "  parent=".(parent::class)."\n";
        echo "  static=".(static::class)."\n";
        echo "    self=".(self::class)."\n";
        echo "--- Calling parent::foo() ----\n";
        parent::foo();
    }
}

(new B())->foo();

outputs:

B::foo() called.
  parent=A
  static=B
    self=B
--- Calling parent::foo() ----
A::foo() called.
  static=B
    self=A

[stronk7]

Hi, I'm closing this as far as it was fixed by PR #32. Thanks all!

[ewallah]

Moodle 3.7 has introduced the coverage.php file. The line return new class extends phpunit_coverage_info { generates a Class extends is not documented error.

So all plugins who use a similar file suffer from the same error.