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 <https://github.com/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
