feat(spec): add MaxAPILevel to ParamSpec for cross-version param tracking

Author: xaionaro@dx.center

tools/cmd/aidl2spec/version_diff.go

@@ -11,27 +11,81 @@ import (
 )
 
 // diffMethodParams compares oldMethod (from oldAPI) with newMethod (from newAPI)
-// and sets MinAPILevel on params that were added in newAPI.
-// Only trailing param additions are detected (AIDL stability guarantees
-// params are never removed or reordered).
+// and annotates params with MinAPILevel / MaxAPILevel to record cross-version
+// differences. The comparison walks params by position:
+//
+//   - Same type at position N → kept as-is (present in both versions).
+//   - Different type at position N → old param gets MaxAPILevel=oldAPI,
+//     new param gets MinAPILevel=newAPI. Both appear in the result.
+//   - Trailing params only in newMethod → MinAPILevel=newAPI.
+//   - Trailing params only in oldMethod → MaxAPILevel=oldAPI.
 func diffMethodParams(
 	oldMethod spec.MethodSpec,
 	newMethod spec.MethodSpec,
 	oldAPI int,
 	newAPI int,
 ) []spec.ParamSpec {
-	result := make([]spec.ParamSpec, len(newMethod.Params))
-	copy(result, newMethod.Params)
+	oldParams := oldMethod.Params
+	newParams := newMethod.Params
 
-	oldCount := len(oldMethod.Params)
-	for i := range result {
-		if i >= oldCount {
-			result[i].MinAPILevel = newAPI
+	minLen := len(oldParams)
+	if len(newParams) < minLen {
+		minLen = len(newParams)
+	}
+
+	var result []spec.ParamSpec
+
+	// Walk positions present in both versions.
+	for i := 0; i < minLen; i++ {
+		if oldParams[i].Type.Equal(newParams[i].Type) {
+			result = append(result, newParams[i])
+			continue
 		}
+
+		// Type changed at this position: emit both variants with version bounds.
+		old := oldParams[i]
+		old.MaxAPILevel = oldAPI
+		result = append(result, old)
+
+		nw := newParams[i]
+		nw.MinAPILevel = newAPI
+		result = append(result, nw)
+	}
+
+	// Trailing params only in the new version.
+	for i := minLen; i < len(newParams); i++ {
+		p := newParams[i]
+		p.MinAPILevel = newAPI
+		result = append(result, p)
 	}
+
+	// Trailing params only in the old version (removed).
+	for i := minLen; i < len(oldParams); i++ {
+		p := oldParams[i]
+		p.MaxAPILevel = oldAPI
+		result = append(result, p)
+	}
+
 	return result
 }
 
+// paramsChanged reports whether old and new param lists differ in length
+// or in the type at any shared position.
+func paramsChanged(
+	old []spec.ParamSpec,
+	new []spec.ParamSpec,
+) bool {
+	if len(old) != len(new) {
+		return true
+	}
+	for i := range old {
+		if !old[i].Type.Equal(new[i].Type) {
+			return true
+		}
+	}
+	return false
+}
+
 // diffBaselineParams parses AIDL files from the baseline 3rdparty directory,
 // converts them to specs, and diffs each interface method's params against
 // the current (new) specs. Any trailing params added in the new API get
@@ -121,7 +175,7 @@ func diffBaselineParams(
 					// is new (handled by ResolveCode returning an error).
 					continue
 				}
-				if len(m.Params) > len(baselineMethod.Params) {
+				if paramsChanged(baselineMethod.Params, m.Params) {
 					m.Params = diffMethodParams(baselineMethod, *m, baselineAPI, newAPI)
 					diffCount++
 				}

tools/cmd/aidl2spec/version_diff_test.go

@@ -8,7 +8,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestDiffMethodParams(t *testing.T) {
+func TestDiffMethodParams_TrailingAddition(t *testing.T) {
 	oldMethod := spec.MethodSpec{
 		Name: "registerClient",
 		Params: []spec.ParamSpec{
@@ -31,9 +31,12 @@ func TestDiffMethodParams(t *testing.T) {
 
 	result := diffMethodParams(oldMethod, newMethod, 34, 36)
 	require.Len(t, result, 5)
-	assert.Equal(t, 0, result[0].MinAPILevel) // appId: present in both
-	assert.Equal(t, 0, result[3].MinAPILevel) // transport: present in both
+	assert.Equal(t, 0, result[0].MinAPILevel)  // appId: present in both
+	assert.Equal(t, 0, result[0].MaxAPILevel)
+	assert.Equal(t, 0, result[3].MinAPILevel)  // transport: present in both
+	assert.Equal(t, 0, result[3].MaxAPILevel)
 	assert.Equal(t, 36, result[4].MinAPILevel) // attributionSource: added in 36
+	assert.Equal(t, 0, result[4].MaxAPILevel)
 }
 
 func TestDiffMethodParams_NoChange(t *testing.T) {
@@ -48,7 +51,9 @@ func TestDiffMethodParams_NoChange(t *testing.T) {
 	result := diffMethodParams(method, method, 34, 36)
 	require.Len(t, result, 2)
 	assert.Equal(t, 0, result[0].MinAPILevel)
+	assert.Equal(t, 0, result[0].MaxAPILevel)
 	assert.Equal(t, 0, result[1].MinAPILevel)
+	assert.Equal(t, 0, result[1].MaxAPILevel)
 }
 
 func TestDiffMethodParams_AllNew(t *testing.T) {
@@ -66,5 +71,265 @@ func TestDiffMethodParams_AllNew(t *testing.T) {
 	result := diffMethodParams(oldMethod, newMethod, 34, 36)
 	require.Len(t, result, 2)
 	assert.Equal(t, 36, result[0].MinAPILevel)
+	assert.Equal(t, 0, result[0].MaxAPILevel)
 	assert.Equal(t, 36, result[1].MinAPILevel)
+	assert.Equal(t, 0, result[1].MaxAPILevel)
+}
+
+func TestDiffMethodParams_ParamRemoval(t *testing.T) {
+	oldMethod := spec.MethodSpec{
+		Name: "configure",
+		Params: []spec.ParamSpec{
+			{Name: "id", Type: spec.TypeRef{Name: "int"}},
+			{Name: "flags", Type: spec.TypeRef{Name: "int"}},
+			{Name: "legacy", Type: spec.TypeRef{Name: "String"}},
+		},
+	}
+	newMethod := spec.MethodSpec{
+		Name: "configure",
+		Params: []spec.ParamSpec{
+			{Name: "id", Type: spec.TypeRef{Name: "int"}},
+		},
+	}
+
+	result := diffMethodParams(oldMethod, newMethod, 34, 36)
+	require.Len(t, result, 3)
+
+	// id: unchanged
+	assert.Equal(t, "id", result[0].Name)
+	assert.Equal(t, 0, result[0].MinAPILevel)
+	assert.Equal(t, 0, result[0].MaxAPILevel)
+
+	// flags: removed (only in old)
+	assert.Equal(t, "flags", result[1].Name)
+	assert.Equal(t, 0, result[1].MinAPILevel)
+	assert.Equal(t, 34, result[1].MaxAPILevel)
+
+	// legacy: removed (only in old)
+	assert.Equal(t, "legacy", result[2].Name)
+	assert.Equal(t, 0, result[2].MinAPILevel)
+	assert.Equal(t, 34, result[2].MaxAPILevel)
+}
+
+func TestDiffMethodParams_TypeChange(t *testing.T) {
+	oldMethod := spec.MethodSpec{
+		Name: "setConfig",
+		Params: []spec.ParamSpec{
+			{Name: "key", Type: spec.TypeRef{Name: "String"}},
+			{Name: "value", Type: spec.TypeRef{Name: "int"}},
+		},
+	}
+	newMethod := spec.MethodSpec{
+		Name: "setConfig",
+		Params: []spec.ParamSpec{
+			{Name: "key", Type: spec.TypeRef{Name: "String"}},
+			{Name: "value", Type: spec.TypeRef{Name: "ParcelableConfig"}},
+		},
+	}
+
+	result := diffMethodParams(oldMethod, newMethod, 34, 36)
+	require.Len(t, result, 3)
+
+	// key: unchanged
+	assert.Equal(t, "key", result[0].Name)
+	assert.Equal(t, "String", result[0].Type.Name)
+	assert.Equal(t, 0, result[0].MinAPILevel)
+	assert.Equal(t, 0, result[0].MaxAPILevel)
+
+	// value (old variant): capped at oldAPI
+	assert.Equal(t, "value", result[1].Name)
+	assert.Equal(t, "int", result[1].Type.Name)
+	assert.Equal(t, 0, result[1].MinAPILevel)
+	assert.Equal(t, 34, result[1].MaxAPILevel)
+
+	// value (new variant): starts at newAPI
+	assert.Equal(t, "value", result[2].Name)
+	assert.Equal(t, "ParcelableConfig", result[2].Type.Name)
+	assert.Equal(t, 36, result[2].MinAPILevel)
+	assert.Equal(t, 0, result[2].MaxAPILevel)
+}
+
+func TestDiffMethodParams_Mixed(t *testing.T) {
+	// Scenario: 4 old params, 3 new params.
+	// Position 0: same type (unchanged)
+	// Position 1: different type (type change)
+	// Position 2: same type (unchanged)
+	// Position 3: only in old (removed)
+	// No trailing additions in new.
+	oldMethod := spec.MethodSpec{
+		Name: "complexMethod",
+		Params: []spec.ParamSpec{
+			{Name: "ctx", Type: spec.TypeRef{Name: "IBinder"}},
+			{Name: "config", Type: spec.TypeRef{Name: "int"}},
+			{Name: "name", Type: spec.TypeRef{Name: "String"}},
+			{Name: "debug", Type: spec.TypeRef{Name: "boolean"}},
+		},
+	}
+	newMethod := spec.MethodSpec{
+		Name: "complexMethod",
+		Params: []spec.ParamSpec{
+			{Name: "ctx", Type: spec.TypeRef{Name: "IBinder"}},
+			{Name: "config", Type: spec.TypeRef{Name: "Bundle"}},
+			{Name: "name", Type: spec.TypeRef{Name: "String"}},
+		},
+	}
+
+	result := diffMethodParams(oldMethod, newMethod, 34, 36)
+
+	// Expected: ctx, config(old), config(new), name, debug(removed) = 5 entries.
+	require.Len(t, result, 5)
+
+	// ctx: unchanged
+	assert.Equal(t, "ctx", result[0].Name)
+	assert.Equal(t, "IBinder", result[0].Type.Name)
+	assert.Equal(t, 0, result[0].MinAPILevel)
+	assert.Equal(t, 0, result[0].MaxAPILevel)
+
+	// config (old variant): capped
+	assert.Equal(t, "config", result[1].Name)
+	assert.Equal(t, "int", result[1].Type.Name)
+	assert.Equal(t, 0, result[1].MinAPILevel)
+	assert.Equal(t, 34, result[1].MaxAPILevel)
+
+	// config (new variant): starts at newAPI
+	assert.Equal(t, "config", result[2].Name)
+	assert.Equal(t, "Bundle", result[2].Type.Name)
+	assert.Equal(t, 36, result[2].MinAPILevel)
+	assert.Equal(t, 0, result[2].MaxAPILevel)
+
+	// name: unchanged
+	assert.Equal(t, "name", result[3].Name)
+	assert.Equal(t, "String", result[3].Type.Name)
+	assert.Equal(t, 0, result[3].MinAPILevel)
+	assert.Equal(t, 0, result[3].MaxAPILevel)
+
+	// debug: removed
+	assert.Equal(t, "debug", result[4].Name)
+	assert.Equal(t, "boolean", result[4].Type.Name)
+	assert.Equal(t, 0, result[4].MinAPILevel)
+	assert.Equal(t, 34, result[4].MaxAPILevel)
+}
+
+func TestDiffMethodParams_MixedWithTrailingAddition(t *testing.T) {
+	// Position 0: same (unchanged)
+	// Position 1: different type (type change)
+	// Position 2: only in new (trailing addition)
+	oldMethod := spec.MethodSpec{
+		Name: "update",
+		Params: []spec.ParamSpec{
+			{Name: "id", Type: spec.TypeRef{Name: "int"}},
+			{Name: "data", Type: spec.TypeRef{Name: "byte", IsArray: true}},
+		},
+	}
+	newMethod := spec.MethodSpec{
+		Name: "update",
+		Params: []spec.ParamSpec{
+			{Name: "id", Type: spec.TypeRef{Name: "int"}},
+			{Name: "data", Type: spec.TypeRef{Name: "ParcelFileDescriptor"}},
+			{Name: "flags", Type: spec.TypeRef{Name: "int"}},
+		},
+	}
+
+	result := diffMethodParams(oldMethod, newMethod, 34, 36)
+
+	// Expected: id, data(old), data(new), flags = 4 entries.
+	require.Len(t, result, 4)
+
+	// id: unchanged
+	assert.Equal(t, "id", result[0].Name)
+	assert.Equal(t, 0, result[0].MinAPILevel)
+	assert.Equal(t, 0, result[0].MaxAPILevel)
+
+	// data (old): capped
+	assert.Equal(t, "data", result[1].Name)
+	assert.Equal(t, "byte", result[1].Type.Name)
+	assert.Equal(t, 0, result[1].MinAPILevel)
+	assert.Equal(t, 34, result[1].MaxAPILevel)
+
+	// data (new): starts at newAPI
+	assert.Equal(t, "data", result[2].Name)
+	assert.Equal(t, "ParcelFileDescriptor", result[2].Type.Name)
+	assert.Equal(t, 36, result[2].MinAPILevel)
+	assert.Equal(t, 0, result[2].MaxAPILevel)
+
+	// flags: trailing addition
+	assert.Equal(t, "flags", result[3].Name)
+	assert.Equal(t, 36, result[3].MinAPILevel)
+	assert.Equal(t, 0, result[3].MaxAPILevel)
+}
+
+func TestDiffMethodParams_GenericTypeArgs(t *testing.T) {
+	// Verify that TypeRef.Equal compares TypeArgs recursively.
+	oldMethod := spec.MethodSpec{
+		Name: "getItems",
+		Params: []spec.ParamSpec{
+			{
+				Name: "items",
+				Type: spec.TypeRef{
+					Name:     "List",
+					TypeArgs: []spec.TypeRef{{Name: "String"}},
+				},
+			},
+		},
+	}
+	newMethod := spec.MethodSpec{
+		Name: "getItems",
+		Params: []spec.ParamSpec{
+			{
+				Name: "items",
+				Type: spec.TypeRef{
+					Name:     "List",
+					TypeArgs: []spec.TypeRef{{Name: "ParcelableItem"}},
+				},
+			},
+		},
+	}
+
+	result := diffMethodParams(oldMethod, newMethod, 34, 36)
+	require.Len(t, result, 2)
+
+	// old variant: List<String> capped
+	assert.Equal(t, "List", result[0].Type.Name)
+	assert.Equal(t, "String", result[0].Type.TypeArgs[0].Name)
+	assert.Equal(t, 34, result[0].MaxAPILevel)
+
+	// new variant: List<ParcelableItem> starts
+	assert.Equal(t, "List", result[1].Type.Name)
+	assert.Equal(t, "ParcelableItem", result[1].Type.TypeArgs[0].Name)
+	assert.Equal(t, 36, result[1].MinAPILevel)
+}
+
+func TestParamsChanged(t *testing.T) {
+	t.Run("equal", func(t *testing.T) {
+		params := []spec.ParamSpec{
+			{Name: "a", Type: spec.TypeRef{Name: "int"}},
+			{Name: "b", Type: spec.TypeRef{Name: "String"}},
+		}
+		assert.False(t, paramsChanged(params, params))
+	})
+
+	t.Run("different_length", func(t *testing.T) {
+		old := []spec.ParamSpec{
+			{Name: "a", Type: spec.TypeRef{Name: "int"}},
+		}
+		new := []spec.ParamSpec{
+			{Name: "a", Type: spec.TypeRef{Name: "int"}},
+			{Name: "b", Type: spec.TypeRef{Name: "String"}},
+		}
+		assert.True(t, paramsChanged(old, new))
+	})
+
+	t.Run("different_type", func(t *testing.T) {
+		old := []spec.ParamSpec{
+			{Name: "a", Type: spec.TypeRef{Name: "int"}},
+		}
+		new := []spec.ParamSpec{
+			{Name: "a", Type: spec.TypeRef{Name: "long"}},
+		}
+		assert.True(t, paramsChanged(old, new))
+	})
+
+	t.Run("both_empty", func(t *testing.T) {
+		assert.False(t, paramsChanged(nil, nil))
+	})
 }