uqbar-project / wollok

Wollok Programming Language
GNU General Public License v3.0
60 stars 16 forks source link

Enhancement #1726 - quick fix - agregar import de un elemento no existente #1827

Closed nicovio closed 5 years ago

nicovio commented 5 years ago

fix-1726

fixes #1726

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 89.789% when pulling a6c5e2fda71689b85d266772ca0b464289ebf181 on enhancement-#1726-QuickFix-agregar-import-de-un-elemento-no-existente into 9452b510edba952d0cec3d06ac4453757f95e814 on dev.

fdodino commented 5 years ago

Bueno @nicovio , le hice el refactor y lo testeé. Había un temita con la biblioteca mirror, que no hace implicit import, por lo tanto no te la iba a encontrar, ahora sí funciona importar de cualquiera de las 3 bibliotecas:

importSystem

Lo que hice fue principalmente unificar todo en WollokLibExtensions, agregué constantes para cada archivo, y queda en un solo lugar

Hay dos temas que me anoto para seguir a futuro:

@npasserini , si te parece ok lo mergeamos a dev para que entre en el release 1.9.0.

nicovio commented 5 years ago

Perfecto @fdodino . Me gusta lo que hiciste.

Con respecto a los temas:

nicovio commented 5 years ago

Estuve probando dividir matchingImports en 2 métodos:

Antes por ejemplo si teníamos new pitt() nos sugería importar actor.* como si tuviera una clase pitt y actor.pitt como si fuera una clase. Eso ya no pasaría.

Otro beneficio que trae esto es que ya no hace falta fijarse si es válido:

Al preguntar si es well known object (o si es clase en el caso de matchingClasses) ya se descartan todos los elementos que no van.

Es sólo una idea, después decime que te parece @fdodino.

fdodino commented 5 years ago

Sí, podría andar, si podés create issues distintos para cada una de las cosas que charlamos (1- diferenciar wko/object literal vs. class, evitando repetir código, 2- eliminar los imports redundantes cuando) y de paso: habría que agregar tests para esta funcionalidad (fijate la jerarquía AbstractWollokQuickFixTestCase)

fdodino commented 5 years ago

Yo ya mergearía este fin de semana el PR como está.

fdodino commented 5 years ago

@nicovio, estoy queriendo hacer un deploy este fin de semana y necesito cerrar el PR, avisame si vas a tocarlo más (yo preferiría que lo hagas en otro branch)

nicovio commented 5 years ago

Por mi ya pueden mergear a dev y después creo esos 2 issues que hablabamos.

@fdodino