Imports más consistentes

nscarcella commented 4 years ago

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:

import p.q
import q.C

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

├ p.wlk
│ └ C
└ t.wtest

Código

// p.wlk

class C { }

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

fdodino commented 4 years ago

Muy bueno el análisis. Lo malo, es que no tengo opiniones fuertes, o la única opinión fuerte que tengo es que para mí la idea de los packages complejizó la manera de importar cosas, por ahí influido por lo que uno conocía (yo en particular no los uso y tampoco los enseño, con trabajar con archivos a nivel raíz me parece suficiente para una materia inicial de Objetos). Estoy de acuerdo con casi todos los planteos (el F de hecho lo planteó Javi hace 4 años, pero bueno... más allá de estar de acuerdo me da paja dedicarle mucho tiempo a un feature que tiene poco uso), me gustaría que en la próxima reunión empecemos hablando de este tema y cerremos una definición, qué te parece?

lspigariol commented 4 years ago

lo unico que tengo para decir (o confesar) es que no sabía que había packages en wollok...

https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail Libre de virus. www.avast.com https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

El vie., 14 de ago. de 2020 a la(s) 09:29, Fernando Dodino ( notifications@github.com) escribió:

Muy bueno el análisis. Lo malo, es que no tengo opiniones fuertes, o la única opinión fuerte que tengo es que para mí la idea de los packages complejizó la manera de importar cosas, por ahí influido por lo que uno conocía (yo en particular no los uso y tampoco los enseño, con trabajar con archivos a nivel raíz me parece suficiente para una materia inicial de Objetos). Estoy de acuerdo con casi todos los planteos (el F de hecho lo planteó Javi hace 4 años, pero bueno... más allá de estar de acuerdo me da paja dedicarle mucho tiempo a un feature que tiene poco uso), me gustaría que en la próxima reunión empecemos hablando de este tema y cerremos una definición, qué te parece?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/uqbar-project/wollok-language/issues/66#issuecomment-674050486, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACZRXG567HXDB4AHLPGNSDLSAUUZFANCNFSM4P626VNQ .

PalumboN commented 3 years ago

Yo volaría los packages y evitaría las colisiones de los imports.

nscarcella commented 3 years ago

Para mi eliminar los packages no es tan piola.

De entrada, no podemos eliminarlos completamente porque los archivos se representan como packages y no me hace muy feliz la idea de tener un concepto del lenguaje que no está representado en la sintaxis y queda solo atado a la representación "física", especialmente si estamos yendo por un Wollok mas web-friendly.

Por poner un ejemplo, sin packages se complica "virtualizar" un ambiente de Wollok en el browser sin acceso a los archivos, o comprimir un programa para minificarlo y moverlo más rápido.

Entiendo que no son un feature super excitante para invertir pero no creo que estemos tan lejos de que sean consistentes y no tenerlos me parece un desperdicio.

uqbar-project / wollok-language