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

Fix #986 #1072

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Fix #986 #1072

wants to merge 16 commits into from

Conversation

manoelcampos
Copy link

@manoelcampos manoelcampos commented Nov 9, 2024

Fix #986

  • Adds test in SettingsTest to confirm the issue of ignoring nested generic type arguments. Considering a class such as List<List<Double>>, the generic type argument is List<Double>, but it was being parsed just as List.
  • Add SettingsTest.testLoadPrimitiveOrRegularClass
  • Adds new tests to CustomTypeMappingTest to confirm the issue of ignoring the generic type argument and mapping every type that matches the raw class name to the mapped type. For instance, if we configure the plugin to map List<BigDecimal> to number[], it was mapping every List attribute to the target type. Even a List<String> was being mapped to number[] in this example.
  • Updates the regex in Settings.parseGenericName. That enables the correct parse of nested generic type arguments. If we have a type such as Class<T1<T2>, T3>, type arguments won't be parsed as T1 and T3 anymore, but as T1<T2> and T3. The Class[T1[T2], T3] syntax also works.
  • Fix the issue of custom type mapping not loading a class that has generic type arguments. The classLoader.loadClass() method requires the raw class name to find the class to load (that is the class name without the generic type arguments). The Settings.loadPrimitiveOrRegularClass now removes the generic parameters from the class name to load the class correctly.
  • Fix the issue in CustomMappingTypeProcessor that was mapping every type that matches the class raw name, ignoring the generic parameters
    ** If we had a mapping List<BigInteger>:number[], every kind of list was being mapped to number[],
    even List<String> and any other kind of list.
    ** Update the CustomMappingTypeProcessor.processType() to filter by the generic type arguments
    after checking the raw class name, to confirm the exact type (such as List<BigInteger> instead of just List)
    before mapping.
    ** The previous change in the default value of Settings.GenericName.typeParameters to an empty list
    makes the filter easier and NPE-safe.
  • Update the ModelCompiler.isValidIdentifierPart() to include more valid chars. Since now the class CustomMappingTypeProcessor correctly extracts each generic type argument (even nested ones), a generic type can have some special chars. For instance, in a type such as java.util.List<java.util.List<String>>, the generic type argument is java.util.List<String>, which has the characters: . < >. Previously, the class was indicating this as an invalid identifier for the type. But this is a valid one, even for Java or TypeScript.
  • Add tests for the new GenericsResolver.typeParameterNameList but I don't know how to make the test work yet.

of ignoring nested generic type arguments.
Considering a class such as List<List<Double>>,
the generic type argument is List<Double>, but
it was being parsed as List.

- Adds toString(), equals() and hashCode() to Settings.GenericName inner class to make tests easier.

Signed-off-by: Manoel Campos <[email protected]>
Use Assertions static import to simplify code

Signed-off-by: Manoel Campos <[email protected]>
- Makes Settings.loadPrimitiveOrRegularClass protected to allow it to be tested

Signed-off-by: Manoel Campos <[email protected]>
of ignoring the generic type argument and mapping every type
that matches the raw class name to the mapped type.

For instance, if we configure the plugin to map List<BigDecimal> to number[],
it will map every List attribute to the target type.
Even a List<String> will be mapped to number[] in this example.

Signed-off-by: Manoel Campos <[email protected]>
…s instead of null

- Storing null required always checking that to avoid NullPointerException,
  making the code dirty.
- Replacing the value by an empty list removes those checks and makes
  it easier to implement the fix for the current issue.

Signed-off-by: Manoel Campos <[email protected]>
That enables the correct parse of nested generic type arguments.
If we have a type such as Class<T1<T2>, T3>, type arguments
won't be parsed as T1 and T3 anymore, but as T1<T2> and T3.

The Class[T1[T2], T3] syntax also works.

Signed-off-by: Manoel Campos <[email protected]>
…eric type arguments

The classLoader.loadClass() method requires the raw class name to find the class to load
(that is the class name without the generic type arguments).

The Settings.loadPrimitiveOrRegularClass now removes the generic parameters from the class
name to load the class correctly.

Signed-off-by: Manoel Campos <[email protected]>
…pe that matches the class raw name, ignoring the generic parameters

- If we had a mapping List<BigInteger>:number[], every kind of list was being mapped to number[],
  even List<String> and any other kind of list.
- Update the CustomMappingTypeProcessor.processType() to filter by the generic type arguments
  after checking the raw class name, to confirm the exact type (such as List<BigInteger> instead of just List)
  before mapping.
- The previous change in the default value of Settings.GenericName.typeParameters to an empty list
  makes the filter easier and NPE-safe.

Signed-off-by: Manoel Campos <[email protected]>
…d chars

Since now the class CustomMappingTypeProcessor correctly extracts each generic type argument
(even nested ones), a generic type can have some special chars.
For instance, in a type such as java.util.List<java.util.List<String>>,
the generic type argument is java.util.List<String>,
which has the characters:   .   <   >

Previously, the class was indicating this as an invalid identifier for the type.
But this is a valid one, even for Java or TypeScript.

Signed-off-by: Manoel Campos <[email protected]>
…n't know how to make the test work yet.

Signed-off-by: Manoel Campos <[email protected]>
It doen't change behaviour.

Signed-off-by: Manoel Campos <[email protected]>
@manoelcampos manoelcampos changed the title Fix #986 (WIP) Fix #986 Nov 10, 2024
Signed-off-by: Manoel Campos <[email protected]>
to include class names that use nested generic types
and check if the correct class/interface reference (type reference)
is loaded.

Signed-off-by: Manoel Campos <[email protected]>
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.

Invalid 'customTypeMapping' format: string>
1 participant