Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [pdt-dev] PDT : Questions about type inference evaluators

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 <thierryblind@xxxxxxx> 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.



Back to the top