join points in AOP must be case sensitive

[vahdat-ab]

Consider the following example:

class A {
  foo;

  after SETFoo {
    // inject
  } 
}

We expect to see a warning because there is no automatic-generated method called SETFoo. However, Umple add injected code into the method setFoo.
Umple doesn't pay attention to lowercase and uppercase letters when it comes to AOP joint points. However, completely we have case sensitive concept in Umple. This must be fixed.

[vahdat-ab]

@Shikib

[Shikib]

I thought this would be a fairly simple change, and it mostly was. I just deleted some code that formatted injection operations/method names when determining relevant code injections for a method.

However, it leads to some tests failing. It seems like it was (at least at one point) intended functionality for Umple to support different naming conventions.

One such test is: cruise.umple.implementation.CodeInjectionTest.SupportUnderscoreNamingToo. An example of something that is expected to work in this test case is:

class Student
{
  lazy immutable id;
  name;
  defaulted type = "None";
  String[] roles;

  before getDefault_type { print "start getDefault_type"; }
  after getDefault_type { print "end getDefault_type"; }
  after get_roles { print "end get_roles"; }
  before numberOf_roles { print "start numberOf_roles"; }
  after numberOf_roles { print "end numberOf_roles"; }
  before indexOf_role { print "start indexOf_role"; }
  after indexOf_role { print "end indexOf_role"; }

}

All of the injections above, are expected (by whoever wrote the test) to inject appropriately. The capitalization of the operation name doesn't match the capitalization of the generated name though indexOfRole vs indexOf_role.

So do we no longer want to support different naming conventions like snake case and pascal case and whatever else? What if there's a naming convention where people capitalize everything after the first word (i.e., setFoo --> setFOO)?

Or should case sensitivity only apply to custom code injections? This would probably make more sense because I think prescribing one naming convention (camel case) onto users of Umple isn't really a good idea -- but it's fair to expect them to maintain consistency between methods they name (custom methods) and injections that they're targeting at said methods.

@vahdat-ab

[TimLethbridge]

There should be consistency between the methods named and the injections targetted. And it seems perhaps only important in custom code.

I guess the reason for the above case is due to ruby where underscores are found.

[Shikib]

Ok, so case sensitivity should definitely apply to custom injections. I currently have this working in my local repo.

Should generated injections be case sensitive as well? I feel like they shouldn't be -- because there exist tests that seem to suggest that we want to support many different naming conventions.

What's happening right now is: to determine whether an injection is targetting a method, the method name and the operation name are formatted to snake case (i.e. setFoo -> set_foo, setFOO -> set_foo, set_Foo -> set_foo). We then check for equality of the formatted names. Removing this formatting causes the aforementioned test cases to fail.

So do we want generated injections to be case sensitive, at the cost of not supporting namking conventions like set_Foo? Or do we want to keep generated injections working in the same way they are right now -- but make custom injections be case sensitive (and in fact identical to the name of the method -- so set_foo and setFoo won't let the injection pass)?

[Shikib]

@TimLethbridge @vahdat-ab can you give me a verdict on this? Do we want case sensitive injections for generated methods.

[TimLethbridge]

My personal take on it is that for Injected code I am fine with case insensitive.

Rationale:
a) That is the way it worked, so it won't break any code.
b) Fewer errors for the user

But the downside is slight inconsistency with what happens for custom code. We can see if @vahdat-ab has any very strong opinions.

[vahdat-ab]

My main concern is inconsistency. This currently makes several issues for other people in the lab. Fewer errors is from the point of view of Umple developer (less failure for test cases). However, if you are a developer for a real system you expect to have solid language and structure for everything. With this unorganized flexibility you end up with a final system which is not what you want. I believe while we keep everything simple but won't let people easy make mistakes. If Umple is case sensitive it should be applied to all parts.
Having case insensitive in this case just bring confusion for designers.

[TimLethbridge]

OK. How many tests fail if we change to be case sensitive; we should do the minimum number of adjustments to the tests.

[Shikib]

There are 9 tests that fail if I change the generated code injections to be case sensitive. Three of them are specifically testing this functionality (of non case sensitivity/supporting different naming conventions) and the others are simply using the other naming conventions functionality (the ruby injection tests).

[Shikib]

It seems like I've been misunderstanding how injections work for other languages. I was under the impression that if the Ruby generator wanted the code injections for the setter for the variable foo it would send a request to getApplicableCodeInjections with the method name setFoo.

However, what actually happens is the generator sends a request with the method name set_foo, the Ruby version of the name. Ultimately, this means one of two things:

(1) enforcing case sensitivity (without changing anything else) would mean that the same Umple code (see example below) would not work for two different languages (e.g., ruby/java).

class A {
  k;

  after setK {
    // test
  } 
}

The above example would have the injection only work for the Java code, because when we want the Ruby injection code -- we would be requesting for the injections into set_k -- and because of case sensitivity, we wouldn't consider the setK injection at all.

(2) The alternative option (to avoid the above issue) is to look through the calls to getApplicableCodeInjections in the generator code and replace the code that converts the name of the method to the formatting of the language with the Java version of the method. (i.e., if we're looking for injections into the setter for variable k, rather than converting the name to the Ruby format, we simply ask for injections into setK).

I'm not sure if this solution will work, but I have a good feeling about it.

I don't think we want option (1) to be the case, as people should be writing Umple without consideration for the target language, but I'm not sure whether (2) is a great option either. Any suggestions?

[TimLethbridge]

So we agreed to option 2