make shouldUseOverrideKeyword ignore initialize method

[juanbono]

Arregla #123
Este PR modifica la validation ya existente shouldUseOverrideKeyword para que no aparezca si el metodo es initialize.

El programa que use para testearlo es (PR en wollok-language):

// This describe should not trigger a shouldUseOverrideKeyword warning
// @Expect(code="shouldUseOverrideKeyword", level="warning")
describe "Fake describe" {
    method initialize() {

    }

    test "2 == 1 + 1" {
        assert.equals(2, 1+1)
    }
}


class Perro {
    // @Expect(code="shouldUseOverrideKeyword", level="warning")
    method initialize() {

    }

    method ladrar() {
        return "guau"
    }
}

Una pregunta que tengo es, hay algun NotExpect? ya que en este caso quiero testear que no aparece la validation.

[fdodino]

Un detalle importante es que hay que

1. hacer el cambio en wollok-language con el test diferente
2. ese test sería bueno que en el initialize haga algo que produzca efecto (para no tener un initialize vacío)
3. una vez que se apruebe ese PR, podemos liberar una versión nueva de language
4. reapuntamos este PR para subirle la versión a language y ahí va a pasar CI
5. se mergea

[fdodino]

Suggested change

	!(!node.isOverride && superclassMethod(node) && !['initialize'].includes(node.name))
	node.isOverride || !superclassMethod(node) || ['initialize'].includes(node.name))

prefiero que haya menos negación y más aserción

[PalumboN]

Agree con vos Fer.

Igual para mí las validations se están declarando mal: afirman cuando el programa es correcto, y para mí debería afirmar cuándo quiero que aparezca el error.... no sé, siempre queda incómodo pensar el booleano.

Otra cosa es que nombre del método debe ser initialize (no incluir), y ese string ya debería estar en algún lado (la VM llama al método cada vez que instancia un objeto), tratemos de usar esa.

[PalumboN]

Ahh ahí vi que el includes es sobre una lista, okkk!

Por qué no usar ==? Igual yo esperaría que ya haya una función isInitialize 🤔

[fdodino]

Arregla #123 Este PR modifica la validation ya existente shouldUseOverrideKeyword para que no aparezca si el metodo es initialize.

El programa que use para testearlo es (PR en wollok-language):

// This describe should not trigger a shouldUseOverrideKeyword warning
// @Expect(code="shouldUseOverrideKeyword", level="warning")
describe "Fake describe" {
    method initialize() {

    }

    test "2 == 1 + 1" {
        assert.equals(2, 1+1)
    }
}


class Perro {
    // @Expect(code="shouldUseOverrideKeyword", level="warning")
    method initialize() {

    }

    method ladrar() {
        return "guau"
    }
}

Una pregunta que tengo es, hay algun NotExpect? ya que en este caso quiero testear que no aparece la validation.

Groso @juanbono !

Es medio una paja todo el ciclo, pero ahí te lo dejé escrito. Por otra parte, no hay un NotExpect, al menos no por ahora. Con que no falle estaríamos bien, salvo que @nscarcella haya desarrollado algo. Cualquier cosa chiflame.

[PalumboN]

Ahí agregué otra cosa al comentario de Fer.

No hay un @NotExpect, los tests del validador fallan si se produce un error en un nodo que no tiene @Expect así que va a funcionar igual.
PEROOOO si lo querés dejar como documentación (cosa que me parece maravillosa) podés simplemente agregarlo! El test debería ignorar esa annotation 😄

[PalumboN]

Agree con vos Fer.

Igual para mí las validations se están declarando mal: afirman cuando el programa es correcto, y para mí debería afirmar cuándo quiero que aparezca el error.... no sé, siempre queda incómodo pensar el booleano.

Otra cosa es que nombre del método debe ser initialize (no incluir), y ese string ya debería estar en algún lado (la VM llama al método cada vez que instancia un objeto), tratemos de usar esa.

[PalumboN]

@juanbono me tomé el atrevimiento de hacer los cambios pendientes para mergear y seguir.

Muchas gracias por tus infinitas contribuciones! 🚀 💯