Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opening a class that does not exist should raise an error #77

Merged

Conversation

echebbi
Copy link
Collaborator

@echebbi echebbi commented Oct 16, 2019

Fix #19.

1. Enhance the type checker to show an error when opening a class that does not exist:

behavior helloworld;

open class Foo {

}

Here, Foo can be:

  • a static class defined within an Ecore metamodel,
  • a runtime class defined in the same .ale file,
  • a runtime class defined in another .ale file.

Otherwise, an error is shown.

2. Enhance the type checker to show a warning when opening a class that has namesakes

behavior helloworld;

open class Foo {

}

Will raise a warning if a class Foo is defined:

  • in a package not called helloworld,
  • in an .ale file which behavior is not helloworld

image

Because:
 - users should not be able to write "open class foo" if foo has not been defined in an Ecore metamodel.

How:
 - test the base class of extended classes in OpenClassValidator
@echebbi echebbi added the affect: validators Related to Xtext validators label Oct 16, 2019
@echebbi echebbi self-assigned this Oct 16, 2019
)

Because:
 - Prevents mistakenly opening the wrong class

How:
 - Check all classes registered in the EMF's package provider having the same name but not the same EPackage.
@echebbi echebbi force-pushed the 19-opening-a-class-that-does-not-exist-should-raise-an-error branch from cb8151b to 68c7793 Compare October 16, 2019 14:06
@echebbi
Copy link
Collaborator Author

echebbi commented Oct 16, 2019

For public record: the code used to check whether the opened class exists is not 100% reliable at the moment. Indeed, when building the AST, if the base class does not exist then the type of the opened class is set to EClassifier by default (see ModelBuilder#resolve(String)).

I first decided to assume that each time an opened class has EClassifier for base class then that would mean that in fact the class does not exist (see OpenClassValidator#validateExtendedClassExists). However some tests are actually opening EClassifier and thus started to fail.

I believe that the proper way to solve this would be to update the resolve method to return an Optional<EClassifier>. The problem is that the current code assumes that ExtendedClass#getBaseClass always returns a valid value which would not be the case anymore. As a result we have to change the code at multiple different places and I currently do not have the time for that.

Copy link
Contributor

@dvojtise dvojtise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@dvojtise
Copy link
Contributor

I fully agree about your comment "resolve method should return an Optional"

the line bellow

clearly states that this is an error case that is not correctly handled.

I suggest opening an issue to track its resolution.

@dvojtise dvojtise merged commit 03fb10b into master Oct 22, 2019
@echebbi echebbi deleted the 19-opening-a-class-that-does-not-exist-should-raise-an-error branch December 8, 2019 01:15
echebbi added a commit that referenced this pull request Jul 3, 2020
Hovering the name of an open class now shows its fully qualified name and details
about the EPackage it comes from.

Because:
 - it's an handy way to check whether we're opening the right class (see #19, #77)

Signed-off-by: Emmanuel Chebbi <emmanuel.chebbi@outlook.fr>
echebbi added a commit that referenced this pull request Jul 3, 2020
Hovering the name of an open class now shows its fully qualified name and details
about the EPackage it comes from.

Because:
 - it's an handy way to check whether we're opening the right class (see #19, #77)

Signed-off-by: Emmanuel Chebbi <emmanuel.chebbi@outlook.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affect: validators Related to Xtext validators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Opening a class that does not exist should raise a warning in the editor
2 participants