Buenas,
Estuve trabajando un poco en el linker de TS y en el proceso saltaron algunas cosas sobre cómo se comportan los packages e imports que me gustaría discutir. No estoy seguro cuántas de estas cosas son bugs, cuántas son así por diseño y cuántas son sólo muy difíciles y no valen la pena, pero yo tiro todo lo que fui encontrando y vamos viendo (y de paso queda la discusión en el proyecto).
Pensé en crear tickets separados, pero creo que se puede poner confuso, así que voy a crear un uber-issue acá para discutir (tratando de ser lo más prolijo posible) y, de ser necesario, podemos ir brancheando tickets a criterio.
Empiezo:
A) Los imports tienen mayor precedencia que las definiciones locales
Si se define en el scope local una entidad con el mismo nombre que otra importada, la entidad importada oculta la local. La versión actual de Wollok-Xtext muestra un warning en la definición local indicando que el nombre ya está definido, pero me parece que de todos modos la definición local debería tapar la importada (de otro modo la definición local es inreferenciable).
Estructura
├ p.wlk
│ └ o
└ t.wtest
└ o
Código
// p.wlk
object o { method m() = "imported" }// t.wtest
import p.*
object o { method m() = "local" }
test "Las definiciones locales sobreescriben a las importadas" {
assert.equals("local", o.m()) // FALLA! o es p.o
}Esto es especialmente choto porque, si en lugar de importar usas referencias calificadas el comportamiento sí es el esperado:
Estructura
├ p.wlk
│ └ C
└ t.wtest
└ p
└ C
Código
// p.wlk
class C { method m() = "imported" }// t.wtest
import p.*
package p {
class C { method m() = "local" }
}
test "Las definiciones locales sobreescriben a las externas" {
assert.equals("local", new p.C().m()) // Esto sí funciona
}Expectativa
El test pasa, porque la definición local de o tiene precedencia por sobre lo importado.
Situación Actual
El test falla, porque o se resuelve al objeto importado.
Propuesta
- Hacer que las definiciones locales tengan precedencia por encima de los imports
- Convertir el
warning en un error (o sea, tratarlo igual que cuando definís dos entidades con el mismo nombre en el mismo package)
B) Se pueden importar packages (pero no sirve para nada)
Actualmente se puede hacer imports de packages o archivos (que yo siempre trato como packages de todos modos), pero ese import no parece cambiar la definición de ninguna forma.
Estructura
├ p.wlk
│ └ q
│ └ C
└ t.wtest
└ D
Código
// p.wlk
package q {
class C { }
}// t.wtest
import p.q.C // OK
import p.q // OK, pero no hace nada?
import q.C // ERROR! No se puede resolver q.C
class D inherits q.C { } // ERROR! No se puede resolver q.C
test "importar packages no hace nada?" {
new C() // OK
new p.q.C() // OK
new q.C() // ERROR! No se puede resolverq.C
}Expectativa
Todas las entidades importadas afectan el namespace local
Situación Actual
Importar packages no parece servir para nada, ya que ninguna referencia calificada puede partir del package importado.
Propuesta
Entiendo que la idea atrás de esto es tener definiciones más declarativas, entonces tiene sentido, por ejemplo, no permitir que un package "vea" al otro, porque esto hace que el orden de los imports sea relevante:
Con lo cual, si la idea es preservar esa declaratividad, propongo agregar una validación que impida importar packages.
Si no, deberíamos agregar el package importado al scope para que cualquier referencia calificada pueda resolverse a partir de él, y hacer que en el test que muestro anden todos los casos, pero si vamos por esto hay que tener cuidado con algunos casos complicados (como el del punto C).
C) Sobrecarga de símbolos
Actualmente se puede tener en el mismo contexto entidades con el mismo nombre, siempre y cuando cumplan "roles" diferentes. Por ejemplo, se pueden tener objects y packages con el mismo nombre, en el mismo nivel.
Para ser justos, esto no causa ninguna ambigüedad (o, al menos, a mi no se me ocurre ninguna), dado que los packages sólo pueden mencionarse en el código como raíz de una referencia calificada y ninguna otra cosa puede ponerse en ese lugar, pero pensé en mencionarlo porque creo que podría generar situaciones confusas.
Un caso fácil de ver es cuando un package contiene algo con su mismo nombre (por ejemplo p.q.p, donde q.p podría ser un objeto). Alguien que hace un import de ese objeto tiene al mismo tiempo una referencia a p package y a p objeto. Como es fácil olvidarse de mirar los imports, alguien podría confundirse aunque no hayan ambigüedades (especialmente si consideramos el problema descrito en (B)).
Estructura
└ t.wtest
├ x (object)
└ x (package)
Código
// t.wtest
package x {
object y { }
}
object x {
const property y = 5
}
test "Sobrecarga de símbolos" {
assert.that(x.y != 5) // Esto funciona, pero podría confundir a alguien que viene de un lenguaje con atributos públicos (Ej: JS).
}Expectativa
Dejarme sobrecargar el namespace es un poco raro...
Situación Actual
Puedo definir objetos y constantes al mismo nivel de packages con el mismo nombre.
Propuesta
Bien podríamos dejar todo tal cual está. No me molesta. Sólo no sé si esto es intencional o simplemente emergente. También podríamos poner un warning. No sé si da poner un error porque, como dije, no produce ambigüedades.
D) Multiples imports a la misma referencia
Esto no es realmente un problema, pero suena a una redundancia que sería prolijo evitar.
Ahora mismo podemos importar la misma entidad tantas veces como queramos y Wollok no se queja.
Estructura
Código
// t.wtest
import p.*
import p.C // Al pedo
import p.C // Doblemente al pedo
import p.* // Al pedo al cubo
Expectativa
Cuando pongo un import redundante alguien me sugiere que pare.
Situación Actual
Podés poner todos los imports que quieras.
Propuesta
Meterle un warning a los imports redundantes.
E) Diferencias entre archivos y packages
Hay algunas diferencias entre los "packages" definidos por un archivo y los packages creados explicitamente por código:
- Los archivos pueden definir constantes a nivel raíz, pero los packages no.
- Los archivos pueden anidarse, pero los packages no.
Capaz se me escapa alguna otra, pero bueno... El punto es que no entiendo para qué tenemos estas restricciones. El metamodelo se hace mucho más simple tratando a cualquier directorio/archivo como un package más. No me parece que se gane nada obligandome a extraer un archivo solamente para poder crear una constante o anidar un package y, si eventualmente le ponemos fichas a un IDE web, podría estar bueno pensar en anidamiento de packages en lugar de estructuras de directorios.
Estructura
├ p.wlk
│ ├ q
│ └ c
└ t.wtest
Código
// p.wlk
const c = 5 // Funciona
package q { } // Funciona// t.wtest
package p {
const c = 5 // ERROR! No se pueden poner constantes en packages.
package q { } // ERROR! No se pueden anidar packages.
}Expectativa
Los archivos y directorios son tratados como packages.
Situación Actual
Los packages son más restrictivos que los archivos.
Propuesta
Permitir a los packages hacer todo lo que hacen los archivos para poder prescindir de la idea de archivo en el metamodelo.
F) Imports ambiguos
Este me parece el caso más fulero de todos. Actualmente no tenemos ninguna manera de referenciar en una expresión dos entidades que se llaman igual. Con lo cual, si necesitás importar dos objects con el mismo nombre de dos packages diferentes la tenés super adentro.
Es más, si importás los dos el compilador no se queja, ignora el segundo import y toma la primer referencia importada.
Estructura
├ p.wlk
│ └ o
├ q.wlk
│ └ o
└ t.wtest
Código
// p.wlk
object o {method m() = "Vengo de P"}// q.wlk
object o {method m() = "Vengo de Q"}// t.wtest
import p.o
import q.o
test "imports ambiguos" {
assert.equals("Vengo de Q", o.m()) // FALLA! o es p.o
assert.equals("Vengo de Q", q.o.m()) // ERROR! no se puede usar una referencia calificada acá
}Expectativa
La duplicidad de importación es una ambigüedad y debería ser marcada como un error. Debería haber una forma de referenciar dos cosas que fueron creadas con el mismo nombre.
Situación Actual
No se marca el error de la ambigüedad en los imports.
No hay forma de hacer convivir los dos objetos o.
Propuesta
A corto plazo: Agregar el error en los imports.
Para 3.0: Extender la sintaxis para incorporar la posibilidad de crear alias para los imports:
import p.o as po
import p.C as PC
Supongo que es discutible, pero me parece un feature bastante simple de implementar que nos ahorraría varios dolores de cabeza.
Perdón, por el issue larguísimo... Le mando un saludo a @fdodino, @npasserini, @PalumboN, mi tía Nelly y todos los que me conocen. La radio está buenísima.
Besis.
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent