PascalCase causes problem

[huan086]

Automatically changing to PascalCase turns out to be a bad idea

1. For classes that implement hashCode, hashCode is marked Obsolete, but Object.hashCode isn't, causing warnings. Similar problem for equals.
2. Inheriting the classes requires that both camelCase and PascalCase be overridden.
3. Inner classes do not get PascalCase. E.g. FunctionValues.ValueFiller
4. Interfaces do not get PascalCase

Turning off PascalCase would be the easiest solution.

If not, there would be the need to ensure that methods inherited from non-FlexLucene classes remain as camelCase (e.g. hashCode, equals). Also, the camelCase methods should call the PascalCase method, instead of implementing twice.

[seemantr]

@huan086 Thanks for reporting this issue. We always knew it was a double edged sword and are aware of the issues you have mentioned. If we don't do pascal case then we have problem with tools like Resharper reporting every method in violation with the standard naming convention and it does not seem like writing idiomatic C# code.

Are you happy to work with me on individual issues and hopefully we can resolve most of these?

For point 1: We can just ignore all the method coming from java.lang.Object (https://docs.oracle.com/javase/8/docs/api/java/lang/Object.html)?

For point 2: Maybe only add new methods for classes marked as final in Java? That way all the abstract classes won't have the method renamed?

For point 3: I tried renaming the inner classes as well but it interferes with the IKVM magic and everything stops working.

For point 4: It is same as 3.

For your final comment I believe you mean that PascalCase methods should call camelCase as the camelCase are coming from ikvm. This can be looked at but will be slightly more involved as the we will have to generate the method body in cecil to make the actual call where as currently we are just copying the original method's IL codes in the newly added method.

I will try to work through the issues you have raised over a period of next 1 month as I think I will have some free time around Christmas. Anyhow I really appreciate you taking time to raise these. Thanks a lot.

[huan086]

If inner classes and interfaces can't be PascalCase, I would rather not have PascalCase at all. It's very inconsistent when there's multiple casing rules in the same library.

I mean camelCase shall call PascalCase. That is, Obsolete methods call non-obsolete methods. That way, there won't be warnings about overriding obsolete methods and no need to put [Obsolete] in the inherited method.

[seemantr]

@huan086 Thanks for your feedback. I have started looking into the above issues. I have changed my correct case generation strategy and using a three pass approach where we rename methods in 2 passes before we feed the jar files into IKVM. Post IKVM dll then goes through another pass of renaming to cleanup few more things.

This approach has resolved a lot of issues like:

1. All interfaces from Lucene have consistent casing.
2. All base classes from Lucene have consistent casing.
3. There are no more duplicate methods as we are fixing the naming on the Java side itself.
4. Most of the inner classes are flattened so that it is a lot cleaner when accessing them from C#.
5. Obsolete methods signatures are nearly gone.
6. Java can have $ character in class names which does not work well in C# do I have also replaced all $ in class names.

Still open issues: There is no easy way to rename methods coming from interfaces present in java.io or java.lang packages. So, for example any class implementing java.io.Closeable will have wrongly cased close method. If I rename it then the contract will be violated and code will not work. The only way to resolve these is to update the methods in IKVM itself.

One easy route that I can take here is to mark the camelCase methods with EditorBrowsable(EditorBrowsableState = EditorBrowsableState.Never) attribute. This will mean that they won't show up in the intellisense. And then I can add a PascalCase method with the same signature which will shadow the original method.

If you wish you can have a look at the new smoke tests which test that the proper PascalCase methods are generated for abstract classes. These are under SmokeTests.fsx under root directory.

I am hoping to finish all this by the end of the year. That will be for Lucene 5.4

Let me know your thoughts around the above changes and again thanks a lot for raising these.

@vladnega , @eladmarg : Adding you two to make you aware of the new changes coming up. This will break few things like there will be no more nested classes so some enums might point to top level namespaces. I am aware that BooleanOccurClause has moved.

[huan086]

Sounds great!

I would say leave the java.io as camelCase. Those implementing Closeable would most likely be AutoCloseable as well. So in C#, we will be wrapping those objects in using and never explicitly call close() anyway.

[seemantr]

Found a cleaner way of fixing the above. So, in case the base class contains a method which requires implementation of a java.lang class like closable then I am adding a new helper method in the base class which calls the abstract method. So, we don't have to call the camelCase method ever. This also resolves the issue with few other methods. So, all in all Mono Cecil rocks :+1:

[huan086]

Upgraded my project. Everything looks good! Thanks for the hardwork.

(ran into another issue though)