From 5d3d51c06bf11e4e27e0445a4287d871f145d2e2 Mon Sep 17 00:00:00 2001 From: v1ll4n Date: Wed, 15 Jul 2026 14:01:22 +0800 Subject: [PATCH] fix(decompiler): unbound method-ref cast for Object-inherited methods + fixUnreachableDefaultThrow Two fixes clearing commons-lang3 tree errors, zero-regression on all 8 jars. 1. Unbound method-ref FI cast for Object-inherited methods on raw JDK receivers (lambdaArgRawJDKReceiverCast, JDEC_LAMBDA_RAW_JDK_RECV_CAST_OFF): When a method reference targets a method inherited from java.lang.Object (toString, hashCode, equals, getClass, clone, finalize) and the receiver is a raw Stream/Optional, javac tries to bind the ref to Object version (0 args) instead of the unbound instance form (1 arg = receiver), causing "invalid method reference". The cast (Function) Method::toString re-targets the SAM so javac uses the unbound form. Gated by: - InstantiatedMtdDesc first param is a non-Object class (unbound instance ref) - method name is in objectInheritedMethodNames (toString/hashCode/equals/etc.) For methods NOT on Object (e.g. MergedAnnotation::withNonMergedAttributes), javac resolves the unbound form correctly and the cast is skipped (it would break downstream type inference). Clears commons-lang3 MethodUtils map(Method::toString). Zero regression: spring stays 28, fastjson2 stays 0. 2. fixUnreachableDefaultThrow (JDEC_FIX_UNREACHABLE_DEFAULT_THROW_OFF): Post-processing fix that removes a throw new RuntimeException() inserted by fixEmptySwitchDefault into an empty default: when the switch is followed by an ACTUAL statement (not a block-closing }). In that case the empty default falls through to the post-switch code, and the throw makes it unreachable ("unreachable statement"). The throw is only removed when the first non-blank line after the switch-closing } does NOT start with } (real statement like continue;), keeping it when the next line is }else{ or } (block exit where the throw is needed). Clears commons-lang3 RandomStringUtils unreachable statement. Zero regression: fastjson2 ToBoolean (switch close followed by }else{) keeps its throw. Load-bearing test: unbound_methodref_object_method_test.go with UnboundMethodRefSeed. Commons-lang3: 2->1 (MethodUtils and RandomStringUtils fixed; NumberUtils missing-return-statement remains as a latent control-flow issue). --- .../decompiler/core/values/expression.go | 77 ++++++++++++++++-- classparser/dumper.go | 47 +++++------ .../regression/UnboundMethodRefSeed.class | Bin 0 -> 1475 bytes .../unbound_methodref_object_method_test.go | 44 ++++++++++ 4 files changed, 138 insertions(+), 30 deletions(-) create mode 100644 classparser/testdata/regression/UnboundMethodRefSeed.class create mode 100644 classparser/unbound_methodref_object_method_test.go diff --git a/classparser/decompiler/core/values/expression.go b/classparser/decompiler/core/values/expression.go index 870524f..10b54ca 100644 --- a/classparser/decompiler/core/values/expression.go +++ b/classparser/decompiler/core/values/expression.go @@ -2169,6 +2169,23 @@ var jdkGenericLambdaReceivers = map[string]bool{ "java.util.Optional": true, } +// objectInheritedMethodNames lists the public no-arg instance methods declared on java.lang.Object. +// When a method reference targets one of these (e.g. `Method::toString`), javac may try to bind it +// to `Object.toString()` (0 args) instead of the unbound instance form (1 arg = receiver), causing +// "invalid method reference" on a raw receiver whose SAM supplies an Object parameter. The cast +// `(Function) Method::toString` re-targets the SAM so javac uses the unbound form. +// For methods NOT on Object (e.g. `MergedAnnotation::withNonMergedAttributes`), javac resolves the +// unbound form correctly (Object has no such method), and a cast would break downstream type +// inference. So the cast is gated to Object-inherited method names only. +var objectInheritedMethodNames = map[string]bool{ + "toString": true, + "hashCode": true, + "equals": true, + "getClass": true, + "clone": true, + "finalize": true, +} + // jdkGenericCtorDiamondClasses is the small set of JDK generic collection classes whose RAW `new X(...)` // in call-receiver position gets the diamond restored by newRecvJDKGenericDiamond. Kept tiny and // unambiguous (concrete, non-anonymous, diamond-inferable under --release 8). @@ -2247,14 +2264,60 @@ func (f *FunctionCallExpression) lambdaArgRawJDKReceiverCast(i int, funcCtx *cla if !ok || cv.Flag != "lambda" { return "" } - // A method reference binds NATURALLY to the raw SAM (it has no explicit parameter types), so - // the parameterized-FI cast is unnecessary for it AND, for SAMs with nested wildcards (Stream. - // flatMap's `Function>`), the cast pins a concrete - // parameterization that defeats javac poly inference ("method flatMap cannot be applied"). - // Only an explicitly-typed lambda body needs the cast to bind. Skip method references. - // (fastjson2 ObjectReaderCreator.toFieldReaderArray `flatMap(Collection::stream)`.) + // A method reference binds NATURALLY to the raw SAM when its impl-method arity matches the + // raw SAM arity (bound instance refs `obj::method`, static refs `Type::method`). But an + // UNBOUND instance method reference (`Type::method` where method is an instance method) + // takes the receiver as its first parameter — its arity is ONE MORE than the raw SAM's, + // so the bare method ref is "invalid method reference" ("actual and formal argument lists + // differ in length"). The source's `(Function) Method::toString` cast + // re-targets the SAM to the correct parameterized FI whose first type arg matches the + // receiver type. So: allow the cast for unbound instance method references (detected by + // the InstantiatedMtdDesc's first param being a concrete class, not Object — a static + // method ref's first param already matches the raw SAM's Object). Skip all other method + // references (bound refs, static refs) AND skip refs whose FI return type is a raw + // `java.util.stream.Stream` (the nested wildcard SAM `Function>` cannot accept a pinned concrete parameterization). + // (fastjson2 ObjectReaderCreator.toFieldReaderArray `flatMap(Collection::stream)` skips; + // commons-lang3 MethodUtils `map(Method::toString)` fires.) if cv.IsMethodRef { - return "" + // Only fire for unbound instance method references whose instantiated method descriptor + // has a first parameter that is NOT java.lang.Object (the receiver type is concrete). + // A static method ref (e.g. `Arrays::stream`, `String::valueOf`) has its first param + // matching the SAM's erased Object parameter, so the bare ref binds naturally. + if cv.InstantiatedMtdDesc == "" || !strings.HasPrefix(cv.InstantiatedMtdDesc, "(") { + return "" + } + // Extract the first parameter type from the instantiatedMethodType descriptor. + desc := cv.InstantiatedMtdDesc[1:] // skip '(' + firstParamEnd := strings.IndexByte(desc, ';') + if firstParamEnd < 0 { + return "" + } + firstParam := desc[:firstParamEnd+1] + // An unbound instance ref's first param is the receiver class (e.g. `Ljava/lang/reflect/Method;`). + // A static ref's first param is the SAM's erased param (e.g. `Ljava/lang/Object;` or a primitive). + // Only fire when the first param is a non-Object class type. + if !strings.HasPrefix(firstParam, "L") || strings.HasPrefix(firstParam, "Ljava/lang/Object;") { + return "" + } + // Only fire when the method name is one inherited from java.lang.Object (toString, + // hashCode, equals, getClass, clone, finalize). These are the methods where javac + // tries to bind the reference to Object's version (0 args) instead of the unbound + // instance form (1 arg = receiver), causing "invalid method reference" on a raw + // Stream whose SAM provides an Object parameter. For other methods (e.g. + // `MergedAnnotation::withNonMergedAttributes`), javac resolves the unbound form + // correctly (Object has no such method), and adding a cast breaks downstream type + // inference (spring AnnotatedElementUtils). Canonical: commons-lang3 + // MethodUtils `map(Method::toString)`. + refStr := cv.String(&class_context.ClassContext{}) + methodName := "" + if idx := strings.LastIndex(refStr, "::"); idx >= 0 { + methodName = refStr[idx+2:] + } + if !objectInheritedMethodNames[methodName] { + return "" + } + // Fall through to the cast logic below. } // (b) the lambda value carries a denotable parameterized functional-interface type. lamType := cv.Type() diff --git a/classparser/dumper.go b/classparser/dumper.go index 0eb2c4f..c284d98 100644 --- a/classparser/dumper.go +++ b/classparser/dumper.go @@ -1235,6 +1235,11 @@ func (c *ClassObjectDumper) DumpClass() (string, error) { // empty, which otherwise leaves a value-returning method without a return on that path. // Kill-switch: JDEC_FIX_EMPTY_SWITCH_DEFAULT_OFF=1. full = fixEmptySwitchDefault(full) + // fixUnreachableDefaultThrow removes a `throw new RuntimeException();` that fixEmptySwitchDefault + // inserted into an empty `default:` when the switch is followed by an ACTUAL statement (not a + // block-closing `}`). In that case the empty default FALLS THROUGH to the post-switch code, and + // the throw makes it unreachable. Kill-switch: JDEC_FIX_UNREACHABLE_DEFAULT_THROW_OFF=1. + full = fixUnreachableDefaultThrow(full) // fixTryBreakReturn moves a hoisted `return ;` back into a `try { break; }` block // inside a do-while(true) loop, where the try's catch clauses require the return's // checked-exception to be throwable. Kill-switch: JDEC_FIX_TRY_BREAK_RETURN_OFF=1. @@ -2595,9 +2600,6 @@ func needsTrailingIncompleteControlFlowThrow(statementList []statements.Statemen return loopConditionIsConstTrue(s.ConditionValue, funcCtx) && loopBodyHasEscapingBreak(s.Body, "", true, funcCtx) case *statements.IfStatement: - // Disabled: the ifElseAllArmsTerminate check is too fragile for deeply nested - // if/else chains with switch fall-through, causing regressions on gson/spring/fastjson2. - // The "missing return statement" on such methods is handled by other post-processing fixes. return false default: return false @@ -5509,14 +5511,16 @@ func fixThrowTypeVarCast(body string) string { return strings.Join(lines, "\n") } -// fixUnreachableSwitchDefaultThrow removes a `throw new RuntimeException();` inserted by -// fixEmptySwitchDefault into an empty `default:` when the switch is followed by more code -// at the same or lesser indent — the empty default falls through to that code, and the -// throw makes it unreachable ("unreachable statement"). The throw is only removed when the -// line after the switch-closing `}` is NOT a `}` at the same or lesser indent (i.e. there IS -// post-switch code that the default should fall through to). Kill-switch: -// JDEC_FIX_UNREACHABLE_DEFAULT_THROW_OFF=1. Canonical: commons-lang3 RandomStringUtils. -func fixUnreachableSwitchDefaultThrow(body string) string { +// fixUnreachableDefaultThrow removes a `throw new RuntimeException();` that fixEmptySwitchDefault +// inserted into an empty `default:` when the switch is followed by an ACTUAL statement (not a +// block-closing `}`). In that case the empty default FALLS THROUGH to the post-switch code, and +// the throw makes it unreachable ("unreachable statement"). The throw is only removed when the +// first non-blank line after the switch-closing `}` does NOT start with `}` (i.e. it is a real +// statement like `continue;`, not a block close like `}else{` or a method-ending `}`). +// Kill-switch: JDEC_FIX_UNREACHABLE_DEFAULT_THROW_OFF=1. +// Keep case: fastjson2 ToBoolean (switch close followed by `}else{` → starts with `}`, keep throw). +// Remove case: commons-lang3 RandomStringUtils (switch close followed by `continue;` → real stmt). +func fixUnreachableDefaultThrow(body string) string { if os.Getenv("JDEC_FIX_UNREACHABLE_DEFAULT_THROW_OFF") == "1" { return body } @@ -5525,7 +5529,7 @@ func fixUnreachableSwitchDefaultThrow(body string) string { // default: // throw new RuntimeException(); // } - // + // defaultRe := regexp.MustCompile(`^(\t+)default:\s*$`) throwRe := regexp.MustCompile(`^\t+throw new RuntimeException\(\);\s*$`) removes := []int{} // line indices to blank out (descending apply) @@ -5565,25 +5569,22 @@ func fixUnreachableSwitchDefaultThrow(body string) string { if switchCloseIdx < 0 { continue } - // Check the line after the switch-closing `}`: if it's NOT `}` at the same or lesser - // indent, there is post-switch code → the throw is unreachable → remove it. + // Check the first non-blank line after the switch-closing `}`. + // If it starts with `}` (block close / `}else{` / `}catch(` etc.) at the same or lesser + // indent, the throw IS needed (the switch is the last statement in its block). + // If it does NOT start with `}`, it is a real post-switch statement → the throw makes it + // unreachable → remove the throw. for k := switchCloseIdx + 1; k < len(lines); k++ { kl := strings.TrimRight(lines[k], "\r") if strings.TrimSpace(kl) == "" { continue } - postIndent := leadingTabs(kl) trimmed := strings.TrimSpace(kl) - if len(postIndent) <= len(indent) && trimmed == "}" { - break // block exit — throw is needed (no post-switch code) - } - if len(postIndent) <= len(indent) { - // Post-switch code exists at the same or lesser indent. - // But only remove the throw if the post-switch code is reachable (not itself - // unreachable). A `return`/`throw`/`continue`/`break` as post-switch code means - // the default falls through to a reachable terminator — the throw is unreachable. + if !strings.HasPrefix(trimmed, "}") { + // Real post-switch statement → the default falls through to it → throw is unreachable. removes = append(removes, throwIdx) } + // Else: starts with `}` → throw is needed (block exit). break } } diff --git a/classparser/testdata/regression/UnboundMethodRefSeed.class b/classparser/testdata/regression/UnboundMethodRefSeed.class new file mode 100644 index 0000000000000000000000000000000000000000..1c566b8e0993f26c5c8d1e3da24c0cb4bbcf413b GIT binary patch literal 1475 zcma)6+foxj5Iqw}7B(x1hyo%P5ilUKctdnCAW_7ss8t4ipCl7l*zCr=P{m*Alfr}L zv*kBg_FjMq1j>Ebnd#~7)93We{Qi6N6To9^sz{+rLAQo9G7R}$zRz`w+uQnw>aM8y z44Dwsar&A$=YHQ#h>eIuC|S@h$Yf&mTJaf4y-g4|SPleZX_3ljSIY!}zo zgCU00{Dg{|7*TLb!zjjx&N;l2V_Bq+<9ZCru0tBx+YCzyZlqvohr1Q=B@lK^7#$!_ zQbg<~G~C7{C9B1m85U&{iG-&yNkXYthJvZK7Lwjnm4X?DflqeT3GDhi;qN&0kD^f# zqR!A=a_WL1S2k_&K4?~jyTz+kXsHXY=9*Oz2N20Axtg%s$CRQ9Vkm>5w_t-!SRojswKd9zyQ#E&<4c=YJrWSjEpj|`dw#d2(NyKV_@s_Zy> zLCcW(kQ5+U+5)!@T0(aJ|IHZk@>p4kBy)*dR>kxMoh9w9Z40+#anBQ;f>#XF=QQa= zPr(Mm*yXPDR>fM-t(&GtF#J-W2MHPa-=)rn(l39s=6!~Xzd0H31( literal 0 HcmV?d00001 diff --git a/classparser/unbound_methodref_object_method_test.go b/classparser/unbound_methodref_object_method_test.go new file mode 100644 index 0000000..fe2085b --- /dev/null +++ b/classparser/unbound_methodref_object_method_test.go @@ -0,0 +1,44 @@ +package javaclassparser + +import ( + "os" + "strings" + "testing" +) + +// TestUnboundMethodRefObjectMethodCastIsLoadBearing pins the lambdaArgRawJDKReceiverCast +// fix for unbound instance method references that target Object-inherited methods (toString, +// hashCode, etc.) on a raw JDK generic receiver (Stream/Optional). When the method name IS on +// Object, javac tries to bind the reference to Object's version (0 args) instead of the unbound +// form (1 arg = receiver), causing "invalid method reference". The cast +// `(Function) Method::toString` re-targets the SAM. When the method name is NOT +// on Object (e.g. `MergedAnnotation::withNonMergedAttributes`), javac resolves correctly and +// the cast is skipped (it would break downstream type inference). Kill-switch: +// JDEC_LAMBDA_RAW_JDK_RECV_CAST_OFF. Real hit: commons-lang3 MethodUtils `map(Method::toString)`. +func TestUnboundMethodRefObjectMethodCastIsLoadBearing(t *testing.T) { + // Build a minimal seed class that has a raw List → stream → map(Method::toString) pattern. + // We use a pre-built seed for determinism. + data, err := os.ReadFile("testdata/regression/UnboundMethodRefSeed.class") + if err != nil { + t.Fatalf("read seed: %v", err) + } + + os.Unsetenv("JDEC_LAMBDA_RAW_JDK_RECV_CAST_OFF") + on, err := Decompile(data) + if err != nil { + t.Fatalf("decompile (fix ON) failed: %v", err) + } + if !strings.Contains(on, "(Function") || !strings.Contains(on, "::toString") { + t.Errorf("fix ON: expected (Function<...>) Method::toString cast, got:\n%s", on) + } + + os.Setenv("JDEC_LAMBDA_RAW_JDK_RECV_CAST_OFF", "1") + defer os.Unsetenv("JDEC_LAMBDA_RAW_JDK_RECV_CAST_OFF") + off, err := Decompile(data) + if err != nil { + t.Fatalf("decompile (fix OFF) failed: %v", err) + } + if strings.Contains(off, "(Function") { + t.Errorf("fix OFF: expected bare Method::toString (no cast), got:\n%s", off) + } +}