This mailing list is for sharing questions and ideas so don't hesitate to use it :)
Michal On Sun, Dec 7, 2014 at 9:52 PM, thierry blind <[email protected]> wrote: > 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 <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
