Hi Michal,
> Yup, from time to time I'm working with inference evaluators, but
I'm not expert and I don't know full
> history:) I'm still gathering
knowledge and most of the time I'm trying to resolve simple bug. Your
> questions are completely correct and reasonable:)
I'm also gathering
knowledge ;) What I really miss in PDT is that code is poorly
commented/documented (but on the other side, I'm not a big fan about
over-commenting code).
>> - why aren't splitted values (using String#split("\\|")) tested if they are
>> empty or not ?
> I'm also not sure, but I can imagine that someone assumed that it wont break
> code.
... or nobody didn't think about those case ;)
> General truth is that there are lots of things to refactor/review,
but we need time for it:) I will try in near > future look at these
classes you mention because CA is one of most important part for users
and making > things more consistent is always good idea:)
Thank you for looking into it, even in a far future :) I wrote you this mail
also to keep a trace about my questions & ideas.
And you're right, it's a long-term work to have a deep look into so many code
and to try to refactor it to gain in quality. Let's just do it (little) step by
step ;)
Thierry.
Date: Sun, 7 Dec 2014 20:20:26 +0100
Subject: Re: PDT : Questions about type inference evaluators
From: [email protected]
To: [email protected]
CC: [email protected]
Hi Thierry,
Yup, from time to time I'm working with inference evaluators, but I'm not
expert and I don't know full history:) I'm still gathering knowledge and most
of the time I'm trying to resolve simple bug. Your questions are completely
correct and reasonable:)
So my (stupid) questions are :
- why do some evaluators look after brackets (MethodReturnTypeEvaluator,
PHPDocMethodReturnTypeEvaluator, PHPDocClassVariableEvaluator) and others not
(ClassVariableDeclarationEvaluator, VariableReferenceEvaluator) ?
I'm not sure. It is possible that when someone were fixing one bug related to
arrays type declaration then were not checking every possible related evaluator
assuming bug is only in this specific case. It needs to be reviewed. - why
aren't splitted values (using String#split("\\|")) tested if they are empty or
not ?
I'm also not sure, but I can imagine that someone assumed that it wont break
code. - why are PHPDocClassVariableEvaluator#getArrayType(String type, IType
currentNamespace, int offset) and
PHPDocMethodReturnTypeEvaluator#getArrayType(String type, IType
currentNamespace, int offset)
different ?
I don't know:) Look below. - so getEvaluatedType(String typeName, IType
currentNamespace) and getArrayType(String type, IType currentNamespace, int
offset) from classes PHPDocClassVariableEvaluator and
PHPDocMethodReturnTypeEvaluator can probably be merged (into a class utility?).
It probably needs to be checked carefully but at first sight you are right and
these method should be merges and extracted to be common for both classes.
General truth is that there are lots of things to refactor/review, but we need
time for it:) I will try in near future look at these classes you mention
because CA is one of most important part for users and making things more
consistent is always good idea:)
Thanks,Michal
On Sun, Dec 7, 2014 at 2:36 AM, thierry blind <[email protected]> wrote:
Hello Michal how are you ?
I've seen that you recently worked on some patch to correct problems with type
inference evaluators (Bug 453737 - Tag @var doesn't work well with array types).
I ran FindBugs some time ago and found some bugs related to the usage of
String#split().
Maybe they are still some pending bugs concerning type inference evaluators,
but I do not feel comfortable enough about these parts of code, so maybe you
could help ;)
What I find strange is the way that some evaluators (from packages
org.eclipse.php.internal.core.typeinference.evaluators.*) are splitting types
using String#split("\\|") and handling brackets (for array type declarations).
So my (stupid) questions are :
- why do some evaluators look after brackets (MethodReturnTypeEvaluator,
PHPDocMethodReturnTypeEvaluator, PHPDocClassVariableEvaluator) and others not
(ClassVariableDeclarationEvaluator, VariableReferenceEvaluator) ?
- why aren't splitted values (using String#split("\\|")) tested if they are
empty or not ?
- why are PHPDocClassVariableEvaluator#getArrayType(String type, IType
currentNamespace, int offset) and
PHPDocMethodReturnTypeEvaluator#getArrayType(String type, IType
currentNamespace, int offset)
different ?
- so getEvaluatedType(String typeName, IType currentNamespace) and
getArrayType(String type, IType currentNamespace, int offset) from classes
PHPDocClassVariableEvaluator and PHPDocMethodReturnTypeEvaluator can probably
be merged (into a class utility?).
I made some (untested) changes and provide them "as is" as attachments, so you
can see what I mean.
I also didn't put them on gerrit because I don't plan to make a patch ;)
Thierry.
_______________________________________________
pdt-dev mailing list
[email protected]
To change your delivery options, retrieve your password, or unsubscribe from
this list, visit
https://dev.eclipse.org/mailman/listinfo/pdt-dev
