Add support for PHP 7.4's typed properties
Details
Diff Detail
- Repository
- R52 KDevelop: PHP Support
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
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.
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.
Hmm, this is odd.
It works correctly for me, and tests pass locally too. I also don't see any failures on jenkins: https://build.kde.org/job/KDevelop/job/kdev-php/
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; //member-variable parseDocComment(node, QStringLiteral("var")); TypeBuilderBase::visitClassStatement(node);
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 TypeBuilder::TypeBuilder() : 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