Add support for PHP 7.4's typed properties

Authored by pprkut on Dec 28 2019, 9:59 AM.



Add support for PHP 7.4's typed properties

Diff Detail

R52 KDevelop: PHP Support
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
pprkut created this revision.Dec 28 2019, 9:59 AM
Restricted Application added a project: KDevelop. · View Herald TranscriptDec 28 2019, 9:59 AM
Restricted Application added a subscriber: kdevelop-devel. · View Herald Transcript
pprkut requested review of this revision.Dec 28 2019, 9:59 AM

Ping :)

I would really like to have this in 5.5, so if there's no review by next weekend I'll merge as is.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 18 2020, 10:18 PM
This revision was automatically updated to reflect the committed changes.

Sorry for the very late comment...
Before this commit, the type of properties were deduced from the assign default value:

Since this commit, the type is <no type> if it is not explicitly specified (either in php7 style or in doc-comment)

Some tests do not pass anymore.

Do you think this new behavior is correct or not ?
I can fix the tests if you think the new behavior is OK.

pprkut added a comment.Apr 9 2020, 3:56 PM

Hmm, this is odd.

It works correctly for me, and tests pass locally too. I also don't see any failures on jenkins:

Can you check if you perhaps have a local commit or change that might cause this?

I have compiled from scratch with kdesrc-build (with cmake-options -DCMAKE_BUILD_TYPE=RelWithDebInfo). The duchain test still doesn't pass.
After some debug, it seems that m_gotTypeFromTypeHint is not always initialized correctly.
The following patch fix the problem for me:

diff --git a/duchain/builders/typebuilder.cpp b/duchain/builders/typebuilder.cpp
index 18e6bcb..c7f147e 100644
--- a/duchain/builders/typebuilder.cpp
+++ b/duchain/builders/typebuilder.cpp
@@ -347,6 +347,7 @@ void TypeBuilder::visitClassStatement(ClassStatementAst *node)
             m_gotTypeFromDocComment = false;
     } else {
+        m_gotTypeFromTypeHint = false;
         parseDocComment(node, QStringLiteral("var"));
pprkut added a comment.Tue, May 5, 2:09 PM

I had a look at this again. Updated to latest master of both kdevelop and kdev-php just to make sure I didn't miss a commit in kdevelop that might have influenced this, but my tests still pass just fine. Furthermore, I don't see what your patch would fix. The change you made is in a code-path that's unaffected by property types, and if there is a crash there it should've triggered just the same before the change.

Can you perhaps post the test output so we can check what test breaks exactly for you?

The test output:

2: FAIL!  : Php::TestDUChain::classMemberVar() 'var->type<IntegralType>()' returned FALSE. ()
2:    Loc: [/home/hmitonneau/kde/src/kdev-php/duchain/tests/duchain.cpp(449)]

Here is a better fix:

diff --git a/duchain/builders/typebuilder.cpp b/duchain/builders/typebuilder.cpp
index 18e6bcb..ccf303b 100644
--- a/duchain/builders/typebuilder.cpp
+++ b/duchain/builders/typebuilder.cpp
@@ -47,6 +47,7 @@ namespace Php
     : TypeBuilderBase()
     , m_gotTypeFromDocComment(false)
+    , m_gotTypeFromTypeHint(false)
     , m_gotReturnTypeFromDocComment(false)

I'm using gcc 9.2.1:

$ /usr/bin/gcc --version
gcc (Ubuntu 9.2.1-9ubuntu2) 9.2.1 20191008