Skip to content

Development Adding a New Static Validation

Jump to bottom
javierfernandes edited this page Jul 14, 2016 · 4 revisions

Intro

Static validations are rules that analyse the code for certain problem and report either an ERROR, WARN or INFO. It is important to understand that this checks are static meaning that they don't actually execute the wollok code.

Another thing to notice is that Wollok is an interpreted language, and this static checks are like an optional program you run in order to detect issue before execution. But still you are able to run a wollok program which have static errors (although it is not the default behavior).

So many times Wollok needs both kind of checks, the statically (for example 'you cannot modify constants') but also a check for runtime.

This page covers static validations. The runtime validation would actually be standard xtend/java code in the interpreter.

Steps

It's really cool to do TDD for this kind of changes. We use XPECT for testing this checks. See this page for more info on how to test.

We will use a concrete example reported in this issue "Using local variables only for assignment and then return" https://github.com/uqbar-project/wollok/issues/746

Overall the steps are:

  1. Add a new XPECT test (a file with wollok code)
  2. Add a check method to the validator
  3. Add internationalization for the message

More on this steps next

Add a new XPECT test

For this we need to create a new file with the wollok extension plus the ".xt" extra extension. In

wollok-dev/org.uqbar.project.wollok.tests/src/org/uqbar/project/wollok/tests/xpect

Currently it is not necessary, but a good practice to follow a convention on the name. The name of the file you define here will be used later for the other steps, so all pieces are "tied" by convention. And we can easily find the tests for a given check.

In my case dontUseLocalVarOnlyToReturn.wlk.xt

The file must be a regular valid wollok file containing example code like:

  • code violating the rule (that we are going to write next)
  • code that don't violate

In the same way as we test for positives and negative cases. To be sure the check is right.

Example:

class A {
	var empleados = []
	var ahorros = 2
	
	method costos(persona, cant_hambiente, cant_pisos) {
	     var costo = empleados.sum { per => persona.presupuesto(cant_hambiente, cant_pisos) } 
	     return costo
	}
	method presupuestoMaximo() {
	    var presupuesto_maximo = 0.20  * ahorros
	    return presupuesto_maximo
	}
	
	method localVarWithoutReturningItDirectly() {
		var a = 23
		return a * 3
	}
	
	method localVarReusedInTheBody() {
		var a = 23
		
		[1,2,3].forEach {i => 
			a += i
		}
		return a
	}
	
}

Now we will add XPECT metadata. First a binding to a java class. Which, for now we will say that it is just a matter of adding this same content for all of your tests (unless you need to extend XPECT).

/* XPECT_SETUP org.uqbar.project.wollok.tests.xpect.WollokXPectTest END_SETUP */
class A {
...

And now the most important part is to write the asserts. Instead of writing code for our tests, we just introduce declarative asserts within the code, as wollok comments. In any place that we expect it to detect an issue (warning or errors).

Example:

		 // XPECT errors --> "Local variable used just to return. Consider removing it" at "costo"
	     var costo = empleados.sum { per => persona.presupuesto(cant_hambiente, cant_pisos) } 
	     return costo

Here we must define what the message is going to be (because we are doing TDD), and in which part of the next line is going to be reported ("costo", the variable name, in our case).

So the format of this comment is important and is like:

// XPECT errors --> "EXPECTED MESSAGE" at "TEXT"

If you run the test now (Right click run as JUnit Test) it should fail.

Add the check method

Check logic is implemented as annotated methods in org.uqbar.project.wollok.validation.WollokDslValidator class.

With the form

	@Check
	@DefaultSeverity(ERROR)
	def methodName(SemanticModelToCheck it) {
		if (condition)
		    report(A_KEY_FOR_THE_MESSAGE, it)
	}
  • @Check indicates that it is a static check. Important. This is XText.
  • @DefaultSeverity: this one is our own. Because we have a mechanism on top of XText validations to automatically make the severity configurable (ERROR, WARNING, INFO, and enabled / disabled them). So you specify the default level. But the user can always change it.
  • methodName: here we follow the convention to use the same name of the file we created for its test.
  • SemanticModelToCheck: you will receive an objecto to test. It will an instance of one of your semantic model (language element) derived from the grammar. For example you can receive an "If" or a "Class" declaration, "VariableReference", etc. You check that !
  • body: if you found a problem then you must report it calling one of the "report" methods passing the offending semantic object.

For our example

	@Check
	@DefaultSeverity(WARN)
	def dontUseLocalVarOnlyToReturn(WVariableDeclaration it) {
		if (isLocalToMethod && onlyUsedInReturn) {
			report("Error", it)
		}
	}

Where isLocalToMethod and onlyUsedInReturn are extension methods. You don't want to write logic here in this class. It is good to implement small extension methods.

As the parameter is called "it", a special xtend name, then you don't need to write it as receptor of the message

isLocalToMethod =  it.isLocalToMethod() = isLocalToMethod(it)

Now run the test again and you will see that it will fail because we have put a hardcoded "Error" message. And also our xpect file expected "errors" and this reports "warnings". You will see an error that can be clicked to se the difference in compare mode.

So in this case we change the "errors" by "warnings" in XPECT

 // XPECT warnings --> "Local variable used just to return. Consider removing it" at "costo"

And also fix the message in the check

report("Local variable used just to return. Consider removing it", it)

Well, not actually you will see that it still fails because we expect the error to be reported in the name of the variable, but it is currently being reported on the whole line. That's because of the way we are calling the report() method.

There are several method overloading

report(message, object): reports the problem in the complete object (variable in this case)
report(message, source, EStructuralFeature feature): this is a way to specify which "part" of the object/rule you want to mark as having the issue.

EStructuralFeature are available for our grammar (autogenerated) as constants in the WollokDSLPackage.Literals class. That's why we have it imported in the validator class

import static org.uqbar.project.wollok.wollokDsl.WollokDslPackage.Literals.*

The constant have a name convention that reflects the elements of the grammar you write. In our case the grammar for variable declaration is

(simplified)

WVariableDeclaration:
	(writeable?='var'|'const') variable=WVariable ('=' right=WExpression)?
;

A modifier then the variable itself and then an optional assignment. So we want to report the problem into something like

WVariableDeclaration.variable

Well, there is a constant for that

WVARIABLE_DECLARATION__VARIABLE

So now the call is

report("Local variable used just to return. Consider removing it", it, WVARIABLE_DECLARATION__VARIABLE)

I18n

It is not good to have the message there hardcoded so we use i18n. For that, we also follow a convention. You need to add a new constant to the class org.uqbar.project.wollok.Messages using the same name we used for the method

public static String WollokDslValidator_DONT_USE_LOCAL_VAR_ONLY_TO_RETURN;

Then you need to add values for the different languages that are defined as properties file

**And in org.uqbar.project.wollok/src/org/uqbar/project/wollok/messages.properties

WollokDslValidator_DONT_USE_LOCAL_VAR_ONLY_TO_RETURN = Local variable used just to return. Consider removing it

And in org.uqbar.project.wollok/src/org/uqbar/project/wollok/messages_es.properties

WollokDslValidator_DONT_USE_LOCAL_VAR_ONLY_TO_RETURN = Esta variable solo se usa para retornarla. Considere eliminarla

And use the constant in the report call

report(WollokDslValidator_DONT_USE_LOCAL_VAR_ONLY_TO_RETURN, it, WVARIABLE_DECLARATION__VARIABLE)

That's it !

Clone this wiki locally