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

Crash when boolean attribute and method clash #1191

Closed
TimLethbridge opened this Issue Jan 13, 2018 · 2 comments

Comments

@TimLethbridge
Member

TimLethbridge commented Jan 13, 2018

The following code results in a crash

class UserInfo {     
     boolean isAdmin;
     public boolean getIsAdmin(){}
}

The stack trace is

Exception java.lang.NullPointerException in
 at cruise.umple.compiler.UmpleInternalParser.analyzeMethod(UmpleInternalParser.java:4453)
   => UmpleInternalParser_CodeClass.ump:2818
 at cruise.umple.compiler.UmpleInternalParser.analyzeClassToken(UmpleInternalParser.java:1909)
   => UmpleInternalParser_CodeClass.ump:257
 at cruise.umple.compiler.UmpleInternalParser.analyzeToken(UmpleInternalParser.java:1229)
   => UmpleInternalParser_Code.ump:253
 at cruise.umple.compiler.UmpleInternalParser.analyzeAllTokens(UmpleInternalParser.java:1192)
   => UmpleInternalParser_Code.ump:222
 at cruise.umple.compiler.UmpleInternalParser.analyzeClass(UmpleInternalParser.java:2813)
   => UmpleInternalParser_CodeClass.ump:1150
 at cruise.umple.compiler.UmpleInternalParser.analyzeClassToken(UmpleInternalParser.java:1765)
   => UmpleInternalParser_CodeClass.ump:114
 at cruise.umple.compiler.UmpleInternalParser.analyzeToken(UmpleInternalParser.java:1211)
   => UmpleInternalParser_Code.ump:238
 at cruise.umple.compiler.UmpleInternalParser.analyzeAllTokens(UmpleInternalParser.java:1165)
   => UmpleInternalParser_Code.ump:198
 at cruise.umple.compiler.UmpleInternalParser.analyze(UmpleInternalParser.java:1066)
   => UmpleInternalParser_Code.ump:105
 at cruise.umple.compiler.UmpleModel.run(UmpleModel.java:1463)
   => Umple_Code.ump:250
 at cruise.umple.UmpleConsoleMain.runConsole(UmpleConsoleMain.java:143)
   => Main_Code.ump:178
 at cruise.umple.UmpleConsoleMain.main(UmpleConsoleMain.java:204)
   => Main_Code.ump:237

@TimLethbridge TimLethbridge added this to the April 2018 for release 1.28 milestone Jan 13, 2018

@TimLethbridge TimLethbridge changed the title from Crash when booleanattribute and method clash to Crash when boolean attribute and method clash Jan 13, 2018

@hugessen hugessen self-assigned this Jan 13, 2018

@hugessen

This comment has been minimized.

Show comment
Hide comment
@hugessen

hugessen Jan 19, 2018

Contributor

The line that breaks in this scenario is:
https://github.com/umple/umple/blob/c5fd40a1f818deebd8d84a0c3f6a9fabe32e2a61/cruise.umple/src/UmpleInternalParser_CodeClass.ump#L2817

This case is trying to throw error code 49, when it should be trying to return error code 1009. #1007 introduced some code to automatically generate getters and setters for any variables declared. When public boolean getIsAdmin(){} in the Umple is interpreted, it conflicts with the auto-generated getter and throws a 49. In this case, a 1009 error would be more accurate, as it indicates a

name conflict with auto-generated getter/setter methods

However, if we reverse the order to

class UserInfo {     
     public boolean getIsAdmin(){}
     boolean isAdmin;
}

The order of compilation is reversed and no error is thrown at all. Both this case and the first case should throw a 1009 error, but neither does, each for different reasons.

Contributor

hugessen commented Jan 19, 2018

The line that breaks in this scenario is:
https://github.com/umple/umple/blob/c5fd40a1f818deebd8d84a0c3f6a9fabe32e2a61/cruise.umple/src/UmpleInternalParser_CodeClass.ump#L2817

This case is trying to throw error code 49, when it should be trying to return error code 1009. #1007 introduced some code to automatically generate getters and setters for any variables declared. When public boolean getIsAdmin(){} in the Umple is interpreted, it conflicts with the auto-generated getter and throws a 49. In this case, a 1009 error would be more accurate, as it indicates a

name conflict with auto-generated getter/setter methods

However, if we reverse the order to

class UserInfo {     
     public boolean getIsAdmin(){}
     boolean isAdmin;
}

The order of compilation is reversed and no error is thrown at all. Both this case and the first case should throw a 1009 error, but neither does, each for different reasons.

@TimLethbridge

This comment has been minimized.

Show comment
Hide comment
@TimLethbridge

TimLethbridge Jan 19, 2018

Member

PR #1007 for Issue #962 introduced the notion of having the generated API methods present in the model. There should be error 1009 reported when somebody writes code manually that clashes with generated API

So getIsAdmin() in this example results from that and message 1009 should be generated. PR #1007 hence has some bugs It also seems that the order (whether dup of generated method or the attribute come first) also makes a difference.

Member

TimLethbridge commented Jan 19, 2018

PR #1007 for Issue #962 introduced the notion of having the generated API methods present in the model. There should be error 1009 reported when somebody writes code manually that clashes with generated API

So getIsAdmin() in this example results from that and message 1009 should be generated. PR #1007 hence has some bugs It also seems that the order (whether dup of generated method or the attribute come first) also makes a difference.

hugessen added a commit that referenced this issue Jan 20, 2018

Fix Issue #1191
- Add if block to properly return error code 1009 when a duplicated method is a getter/setter
- Rename verifyIfMethodIsConstructorOrGetSet to isConstructorOrGetSet
- Refactoring isConstructorOrGetSet (was returning opposite values of what it was supposed to, and had unintended side effects)
- isConstructorOrGetSet method will require further refactoring, as it doesn't adhere to single responsibility principle

hugessen added a commit that referenced this issue Jan 27, 2018

Fix Issue #1191
- Add if block to properly return error code 1009 when a duplicated method is a getter/setter
- Rename verifyIfMethodIsConstructorOrGetSet to isConstructorOrGetSet
- Refactoring isConstructorOrGetSet (was returning opposite values of what it was supposed to, and had unintended side effects)
- isConstructorOrGetSet method will require further refactoring, as it doesn't adhere to single responsibility principle

hugessen added a commit that referenced this issue Jan 27, 2018

#1191: Fixed failing tests and added new test case
- Discovered that 1009 error should not be thrown when the user-generated getter/setter has a method body (it is assumed the user did so intentionally) and simply appends the body to the API-generated method
- Wrote tests to cover cases where user-generated method has and does not have a method body

User manual improvements automation moved this from To do to Done Feb 2, 2018

TimLethbridge added a commit that referenced this issue Feb 2, 2018

Merge pull request #1210 from umple/issue_1191
Fixes #1191 Solve conflict with API-generated getter/setter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment