fix(decompiler): unbound method-ref cast for Object methods + fixUnreachableDefaultThrow

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, String>) 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<? super T,? extends Stream<? extends R>>`), 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, String>) 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<? super T, ? extends
+	// Stream<? extends R>>` 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()

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 <expr>;` 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 {
 	//   <indent>default:
 	//   <indent+1>throw new RuntimeException();
 	//   <indent>}
-	//   <indent or less><code that is NOT just `}`>
+	//   <indent><NOT starting with }>
 	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
 		}
 	}

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, String>) 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)
+	}
+}