Inline @SuppressWarnings

Classic Servoy
1

Issue: Jira

@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

/** @SuppressWarnings(wrongparameters) */
scopes.generic.foundsetProcess(foundset);
2

+1

3

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

4

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

/**
 * 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”.

5

Hi James,

there’s actually a way to get rid of the buildmarker, but you can debate on the solution:

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… ;-)

6

ok, so this looks to me a bug: since JSRecord is less specific than JSRecorddb:/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

7

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

+1

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

Done

8

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

/**
  * @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.

9

jcompagner:
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 ;-) )

10

no you are comparing it wrong…

you seem to talk about this scenario:

/** 
  * @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:

/** 
  * @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” , compared to “Array” 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} 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.

11

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.