usethesource / rascal-language-servers

An LSP server for Rascal which includes an easy-to-use LSP generator for languages implemented in Rascal, and an interactive terminal REPL.
BSD 2-Clause "Simplified" License
14 stars 8 forks source link

data-type prefix resolution for constructors depends on order of declaration in source file #261

Open jurgenvinju opened 1 year ago

jurgenvinju commented 1 year ago

Describe the bug

It seems this is an NYI in the type-checker:

image

The feature that the interpreter implements is that DataType::constructorName implements a lookup of the constructorName limited to the definitions of the given data type (or syntax type). It is used to disambiguate when constructorNames are overloaded between different data types in the same scope, and some people use it for documentation purposes even when the call is not ambiguous.

Note that the type checker now warns in case of overloaded constructors. I think that should be an info; it is pretty common and in the YAML, JSON and XML case almost unavoidable to reuse constructor names between different data-types.

PaulKlint commented 1 year ago

This is implemented in the typechecker. A repo case would be helpful so that I can see what is going on.

DavyLandman commented 1 year ago

We are working with a old version of the rascal-core (v0.7.8 from April 2022), so if it's a new feature, that could explain this difference.

jurgenvinju commented 1 year ago

@PaulKlint; of course, isolation is the way forward.

In this simple test case, it seems to be isolated to order to declaration:

module vis::X

data CytoStyle
    = cytoNodeStyle(
        CytoTextWrap \text-wrap = CytoTextWrap::none() // Undefined variable, constructor or function `CytoTextWrap::none`
    )
    ;

data CytoTextWrap = none();

while the following module appears to type-check correctly:

module vis::X

data CytoTextWrap = none();

data CytoStyle
    = cytoNodeStyle(
        CytoTextWrap \text-wrap = CytoTextWrap::none()
    )
    ;
jurgenvinju commented 1 year ago

@DavyLandman ; you're right, we also have that going on. But I don't remember why we could not release the latest version. Do you? Need that information to make decisions about where to go from here.

jurgenvinju commented 1 year ago

@PaulKlint it seems unrelated to keyword fields:

module vis::X

value x = CytoTextWrap::none(); // Undefined variable, constructor or function `CytoTextWrap::none`

data CytoTextWrap = none();
jurgenvinju commented 1 year ago

at least vis::Graphs can compile now by re-ordering the definitions. That's good.

jurgenvinju commented 1 year ago

confirmed: re-ordering in the vis::Graphs module works around all the problems caused by the current issue: https://github.com/usethesource/rascal/actions/runs/5177382094

DavyLandman commented 1 year ago

Should this issue be moved to the rascal project?