-
-
Notifications
You must be signed in to change notification settings - Fork 5
Add simple_subtype_field_names NameProposer
#21
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
base: master
Are you sure you want to change the base?
Add simple_subtype_field_names NameProposer
#21
Conversation
OroArmor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be better as an extension to the simple type field name that already exists, with maybe "inherit_suffix": "Entity" or something.
I'm not sure how that would work. |
|
I guess they could be combined and just be doing effectively two separate things. |
|
Looking into an issue I found by trying the |
implement regex replacement subtype proposer fix SimpleTypeSingleIndex bug that used constant's local names for conflict detection fix SimpleTypeSingleIndex bug that could give outer class fields an inner class as a parent (no known consequences)
log errors instead of throwing during "inherit" parsing
add tests for duplicate subtype fields+locals
dcc29c1 to
6b11416
Compare
1216abb to
e4a40c9
Compare
|
diffs for each of the examples in the OP; some spooky diffs are mixed in |
i agree, i think it's a little cleaner to keep them separate |
extract SimpleSubtypeFieldNameProposerTest add TODO for failing simple subtype duplicate test add TODO for flaky simple type verification test
…gleIndex conflict prevention
add tests for overriding inheritted subtype rules move+fix isValidJavaIdentifier check for SimpleSubtypeFieldNameProposer add tests for not proposing invalid subtype names
|
I've made substantial changes, so new diffs: |
set type verification to THROW for simple sub/type test input
ix0rai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great, this feature is really well done! just wondering why you reimplemented toLowerCase() lol
src/main/java/org/quiltmc/enigma_plugin/proposal/SimpleSubtypeFieldNameProposer.java
Outdated
Show resolved
Hide resolved
...in/java/org/quiltmc/enigma_plugin/index/simple_type_single/SimpleTypeFieldNamesRegistry.java
Outdated
Show resolved
Hide resolved
| * @return a breadth-first stream of the passed {@code classEntry}'s ancestors; | ||
| * order within one level of depth is not guaranteed | ||
| */ | ||
| public static Stream<ClassEntry> streamAncestors(ClassEntry classEntry, InheritanceIndex inheritance) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this implementation does not go in breadth-first order
QuiltMC/enigma#347 will fix this
converting to draft until 347 can be included
Adds the option of naming simple types' subtypes based on a transformation of the super simple type's name.
Heavily based on the simple types proposer.
example use case:
Entitysubclasses with suffixEntity->PiglinEntityfields and params are namedpiglinAdditional fixes
SimpleTypeSingleIndex:ConflictFixProposerlooked up mappings using deobf params, breaking it for deobf method param conflicts which can arise fromDelegateParametersNameProposerFormat
Uses the same registry as simple types and expands
"inherit"to allow object values. Inherit can now take any of the following forms (also see schema):removed"inherit": trueTodo:
LivingEntitys aren't namedliving)