Inline @SuppressWarnings

Discuss all feature requests you have for a new Servoy versions here. Make sure to be clear about what you want, provide an example and indicate how important the feature is for you

Inline @SuppressWarnings

Postby jgarfield » Wed Apr 29, 2015 4:00 pm

Issue: https://support.servoy.com/browse/SVY-8196

@SuppressWarnings at a function level is rarely useful.

More often, the warning you want to suppress is a very specific line of code that is giving you a warning that you know to be invalid.

For example, if there is a call to a function that is shows a "The function X is not applicable for arguments..." warning, if the developer know the call is valid, they can suppress this warning by adding a "@SuppressWarnings(wrongparameters)" annotation to the function header.

This however, is terrible, because now any future changes to this method that may also have invalid parameters, will not be caught by the checker, and could very well be a bug. And as such, it is dangerous to use that kind of suppression at a function level.

On the other hand, you are left with not adding function level suppression so that you are sure to always see appropriate warnings. This of course litters your warnings list with thousands of false-positive warnings that you cannot suppress for want of making sure you see proper warnings. This severely decreases the utility of having a warnings list in the first place because the noise-to-signal ratio is off the charts.

If the developer had the ability to place Inline @SuppressWarnings, this would solve the majority of these issues. I imagine it would work something like inline type annotations would work

Code: Select all
/** @SuppressWarnings(wrongparameters) */
scopes.generic.foundsetProcess(foundset);
Programmer.
adBlocks
http://www.adblocks.com
jgarfield
 
Posts: 223
Joined: Wed Sep 28, 2005 9:02 pm
Location: Boston, US

Re: Inline @SuppressWarnings

Postby ROCLASI » Wed Apr 29, 2015 5:43 pm

+1
Robert Ivens
SAN Developer / Servoy Valued Professional / Servoy Certified Developer

ROCLASI Software Solutions / JBS Group, Partner
Mastodon: @roclasi
--
ServoyForge - Building Open Source Software.
PostgreSQL - The world's most advanced open source database.
User avatar
ROCLASI
Servoy Expert
 
Posts: 5438
Joined: Thu Oct 02, 2003 9:49 am
Location: Netherlands/Belgium

Re: Inline @SuppressWarnings

Postby pbakker » Wed Apr 29, 2015 8:03 pm

While is can be handy in very limited situations, I think most developers will just use it to get rid of warnings, instead of fixing the underlying problem in their code.

In your case, why are you getting a "The function X is not applicable for arguments..." warning? Wouldn't it be better to get proper support for typing the function if you can't properly type it currently?.

Paul
pbakker
 
Posts: 2822
Joined: Wed Oct 01, 2003 8:12 pm
Location: Amsterdam, the Netherlands

Re: Inline @SuppressWarnings

Postby jgarfield » Wed Apr 29, 2015 10:26 pm

Those same developers will just use the function level @SuppressWarnings to get rid of their warnings, and as with anything else, their development practices aren't really my concern.

In my case, I don't know that it is possible to correctly type my code, at least not without some significant work (at least that's how it's been explained to me)

Consider the following
Code: Select all
/**
* Runs a function on records in the foundset while that function returns false. Returns the first record evaluates to true
* @param {JSFoundSet} fs
* @param {function(JSRecord):Boolean} f
*
* @return {JSRecord}
*/
function first(fs, f) {
   if (!(fs && f)) {
      throw new Error("Missing parameters")
   }
   
   for (var i = 1, nSize = fs.getSize(); i <= nSize || i <= (nSize = fs.getSize()); i++) {
      var r = fs.getRecord(i);
      if ( f(r) ) {
         return r;
      }
   }
   
   return undefined;
}

/**
* @param  {JSRecord<db:/app/contact>}  rContact
* @return {Boolean}
*/
function hasGmailAddress(rContact) {
   if (!rContact.email) {
      return false;
   }
   
   return rContact.email.indexOf('@gmail.com') > -1;
}

function tryIt() {
   var fsContact = scopes.contact.getMyContacts();
   var rGmailContact = first(fsContact, hasGmailAddress);
}


In `tryIt` there will be a warning on the call to `first` because `function(JSRecord<db:/app/contact>):Boolean` doesn't match `function(JSRecord):Boolean`. There really isn't a way to correctly annotate the type of JS with the current state of JSDocs once you start dealing with higher level functions, but there is a way for me as a developer to say "yah, that's right".
Programmer.
adBlocks
http://www.adblocks.com
jgarfield
 
Posts: 223
Joined: Wed Sep 28, 2005 9:02 pm
Location: Boston, US

Re: Inline @SuppressWarnings

Postby mboegem » Thu Apr 30, 2015 1:25 am

Hi James,

there's actually a way to get rid of the buildmarker, but you can debate on the solution:
Code: Select all
function tryIt() {
   var fsContact = scopes.contact.getMyContacts();
   /** @type {function(JSRecord):Boolean} */
   var f = hasGmailAddress;
   var rGmailContact = first(fsContact, f);
}


So in the above code I introduce a new variable 'f', typecast that to what the 'first' function expects and then pass the variable instead of the function as the 2nd argument.
Nice solution? Maybe not.
Does it work? Yes.

Like I already stated: you can debate on the solution.... ;-)
Marc Boegem
Solutiative / JBS Group, Partner
• Servoy Certified Developer
• Servoy Valued Professional
• Freelance Developer

Image

Partner of Tower - The most powerful Git client for Mac and Windows
User avatar
mboegem
 
Posts: 1743
Joined: Sun Oct 14, 2007 1:34 pm
Location: Amsterdam

Re: Inline @SuppressWarnings

Postby pbakker » Thu Apr 30, 2015 9:14 am

ok, so this looks to me a bug: since JSRecord is less specific than JSRecord<db:/xxx/xxx>, you shouldn't get a warning here, according to me, since you're supplying first with a function that takes a JSRecord, just a more specific JSRecord. If you file a bug for this, let us know and I'll make sure to vote on it in Jira.

Secondly, to make Marc's suggestion nicer, we'd need inline typing, as suggested in case https://support.servoy.com/browse/SVY-6100. That would eliminate the need for having to create variables just so you can type stuff and get rid of warnings. If you like to see this one in Servoy, make sure to cast your vote

Paul
pbakker
 
Posts: 2822
Joined: Wed Oct 01, 2003 8:12 pm
Location: Amsterdam, the Netherlands

Re: Inline @SuppressWarnings

Postby mboegem » Thu Apr 30, 2015 9:42 am

pbakker wrote:If you file a bug for this, let us know and I'll make sure to vote on it in Jira

+1

pbakker wrote:If you like to see this one in Servoy, make sure to cast your vote

Done
Marc Boegem
Solutiative / JBS Group, Partner
• Servoy Certified Developer
• Servoy Valued Professional
• Freelance Developer

Image

Partner of Tower - The most powerful Git client for Mac and Windows
User avatar
mboegem
 
Posts: 1743
Joined: Sun Oct 14, 2007 1:34 pm
Location: Amsterdam

Re: Inline @SuppressWarnings

Postby jcompagner » Mon May 04, 2015 10:44 am

hmmm

i explained it already fully to James why that is NOT A BUG :)

that code is NOT valid, yes at first sight you think it is but it isn't, with plain java generics this also will fail (we only there have a bit more stuff like "untyped" T, or ? extends T what you can use in specific scenarios)

but i will try to explain it here as simple as possible

Code: Select all
/**
  * @param {function(JSRecord):Boolean} f
  */
function iDoCallTheFunctionThatIGet(f) {
    // f gives me a function where i can pass any record to so i just give it a record of db:/mydb/contacts
   /* @type {JSRecord<db/mydb/concats} */
    var record = ....;
    f(record);
    // so the function is now called with a mydb/contacts record
}

/**
  * @param {JSRecord<db/mydb/orders>}
  */
function iWillBeCalledInTheAboveFunction(r) {
  // i epxect the r to be of type mydb/orders
  var orderid = r.orderid;
  // orderid should be there because this is a record that has to be from orders....
}

function iWillCallTheFunctonWithTheFunctionParam() {
  // here i pass the function that ONLY accepts a mydb/orders types record to the method that accepts all records and will call it later on..
   iDoCallTheFunctionThatIGet(iWillBeCalledInTheAboveFunction);
}


so follow the flow here... you see that it will go wrong in code...
because the function that only accepts a certain type of a JSRecord now get any type (in this example the contacts record) of a JSRecord because it is "up casted"

The other way around will work..
So if the iDoCallTheFunctionThatIGet function would be type as JSRecord<db/mydb/orders>
and iWillBeCalledInTheAboveFunction is not typed, so just JSRecord, because the iWillBeCalledInTheAboveFunction will always just get a orders record but it accepts them all so it doesn't matter.
Johan Compagner
Servoy
User avatar
jcompagner
 
Posts: 8829
Joined: Tue May 27, 2003 7:26 pm
Location: The Internet

Re: Inline @SuppressWarnings

Postby mboegem » Mon May 04, 2015 1:47 pm

jcompagner wrote:The other way around will work..
So if the iDoCallTheFunctionThatIGet function would be type as JSRecord<db/mydb/orders>
and iWillBeCalledInTheAboveFunction is not typed, so just JSRecord, because the iWillBeCalledInTheAboveFunction will always just get a orders record but it accepts them all so it doesn't matter.


hmmm that's the world upside down.

So you're saying:
- I call a function expecting type 'Fruit'
- I do call it with a function passing on 'Banana'
- Servoy will complain that Banana is not of type Fruit.

But the other way around (your explanation)
- I call a function expecting type 'Banana'
- I do call it with a function passing on 'Fruit'
- Servoy will not complain...

Like Paul already wrote 'Fruit' is less specific, so it should just accept 'Banana', 'Orange', 'Apple' (especially the last one is your favourite, I know ;-) )
Marc Boegem
Solutiative / JBS Group, Partner
• Servoy Certified Developer
• Servoy Valued Professional
• Freelance Developer

Image

Partner of Tower - The most powerful Git client for Mac and Windows
User avatar
mboegem
 
Posts: 1743
Joined: Sun Oct 14, 2007 1:34 pm
Location: Amsterdam

Re: Inline @SuppressWarnings

Postby jcompagner » Mon May 04, 2015 2:46 pm

no you are comparing it wrong..

you seem to talk about this scenario:

Code: Select all
/**
  * @param {Fruit} f
  */
function myfunction(f) {
  f.doSomethingThatFruitHas();
}

function caller() {
/* @type {Banana} */
var banana = ....
myfunction(banana)
}



that of course works fine! because banana is a subtype of fruit so it fits..

But look very carefully at the above scenario. That is different then the one in this post!

Because there we have a function that has parameter that have types and that function pointer is passed around, not just the Banana object.

another example with Banana en Fruit which is comparable with the first:

Code: Select all
/**
  * @param {Array<Fruit>} array
  */
function myfunction(array) {
  /* @type {Apple} */
  var apple  = .....
  array.add(apple);
}

function caller() {
/* @type {Array<Banana>} */
var banana = ....
myfunction(banana);
  for(var i = 0; i<banana.length;i++) {
    banana.get(i).aBananaFunctionThatAppleDoesntHave();
  }
}



code BANGS!!

do follow the code here again,

1> caller creates a Banana types Array (so it expects that always Banana's are in that Array)
2> it passes it to the myfunction function which treats the array just a Fruit, so it just adds a Apple
3> now back in the caller method the array is 1 bigger and the caller still thinks that everything is a banana and walks over them to call a specific banana function on it.. But suddenly it encounters an Apple!

This is kind of the same as the one with the function
You seem to think that a type "Banana" , compared to "Fruit", and type "Array<Banana>" , compared to "Array<Fruit>" are kind of the same, the same rules apply, that's not correct those are not the same

the difference in this example is that this one can't be reversed because that also wouldn't fit.. (but then it error would happen in the myfunction function not in the caller function)

in java for the above example we have a solution, the array is made readonly because instead of that you declare the array as:

@param {Array<Fruit>} array

you declare it as:

@param {Array<? extends Fruit>} array

where you say, give me an array where the type thats in the array can a Fruit (so subtypes like bananas)
but what happens then that you only can get it var x = array.get(1); you can't add it: array.add(x) because the compiler of java doesn't know exactly what you can put in it.
Johan Compagner
Servoy
User avatar
jcompagner
 
Posts: 8829
Joined: Tue May 27, 2003 7:26 pm
Location: The Internet

Re: Inline @SuppressWarnings

Postby jgarfield » Mon May 04, 2015 3:11 pm

Sorry Johan.... I did try to make this about @SuppressWarnings..... :D

I do see how the inline typing suggestion in https://support.servoy.com/browse/SVY-6100 would solve this specific function issue, and that wouldn't be a bad solution for this, and inline typing would be a nice thing in other places.


That being said, I still think that @SuppressWarnings at a function level has limited usefulness because of my previously mentioned issues of scope, and that being able to specify on a line level which warnings I'd like to ignore would also offer a whole category of utility.
Programmer.
adBlocks
http://www.adblocks.com
jgarfield
 
Posts: 223
Joined: Wed Sep 28, 2005 9:02 pm
Location: Boston, US


Return to Discuss Feature Requests

Who is online

Users browsing this forum: No registered users and 8 guests