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

Preserve array square bracket pairs when parsing type names #24

Conversation

willibrandon
Copy link
Contributor

Fixes #11

Proposed changes

  • Fix the type name parser tokenize pattern to also match on left and right square brackets so that array square bracket pairs may be preserved during conversion.

Customer Impact

  • Array square bracket pairs will now be preserved during conversion.

Regression?

  • No

Risk

  • Could break people who expect the array square brackets to get dropped but this is unlikely and not something that should be considered breaking.

Test Methodology

  • Ran unit tests and the previously failing test ConvertType_IntArray_BracketsAfterType() is now passing.

@willibrandon
Copy link
Contributor Author

I also investigated using embedded resources in xUnit tests and have that working locally now. It will be a good way to test.

@paulirwin paulirwin merged commit 13a1c98 into paulirwin:master Jul 4, 2021
@paulirwin
Copy link
Owner

@willibrandon Awesome, feel free to submit a new PR for that. Merged this one. Also I noticed that the unit test checks to see if string is translated as string, but string is not a valid type keyword in Java. String will be translated as String, likewise for Object, although I'd like a config option to support translating to the type keywords instead. I'll create another issue for that.

@paulirwin
Copy link
Owner

I was misremembering, we already do that for String, just not Object. Working on that now.

@willibrandon willibrandon deleted the fix_11_PreserveArraySquareBracketPair branch July 4, 2021 03:17
@OJacot-Descombes
Copy link
Contributor

A common convention for type names is to use the C# alias in declarations and the .NET type name when accessing static members.

int i = Int32.MaxValue;

See also: C# Coding Standards and Naming Conventions

@paulirwin
Copy link
Owner

@OJacot-Descombes Currently we do not translate the right-hand side of that statement at all, as we can't syntactically tell the difference between member/property access and static access on a type without symbol resolution. (It's a lot easier for field/parameter/etc types.) For example:

class Foo {
    void bar() {
        int i = Integer.MAX_VALUE;
    }
}

gets translated almost as-is to:

class Foo
{
    virtual void Bar()
    {
        int i = Integer.MAX_VALUE;
    }
}

While we could look into this for some common static methods/properties (i.e. I'd like to see this for #22), we'd need full symbol resolution to cover every case. However, if we wanted to do a "best-guess" translation, Java does make this easier in some regards with their naming conventions, since types would be the only ones with upper-case names for code that follows the convention. I'd be happy to accept a PR for this if you're interested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arrays converts just Normal Data Type
3 participants