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 0000000..1c566b8 Binary files /dev/null and b/classparser/testdata/regression/UnboundMethodRefSeed.class differ 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) + } +}