Skip to content

Commit 20be013

Browse files
abhinavgautam01SteveSandersonMSCopilot
authored
Fix Python codegen synthetic permission approval names (#1652)
* Fix Python codegen synthetic permission approval names * Address Python codegen cleanup review feedback * Regenerate python rpc after rebase Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Replace brittle python codegen symbol test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent e5cf2b6 commit 20be013

3 files changed

Lines changed: 122 additions & 171 deletions

File tree

python/copilot/generated/rpc.py

Lines changed: 0 additions & 171 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

python/test_codegen_type_names.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import re
2+
import types
3+
4+
from copilot.generated import rpc
5+
6+
7+
def test_permission_approval_exports_are_union_aliases():
8+
approval_exports = [
9+
name
10+
for name in rpc.__all__
11+
if re.fullmatch(r"PermissionDecisionApproveFor.*Approval", name)
12+
]
13+
assert approval_exports
14+
15+
for name in approval_exports:
16+
exported = getattr(rpc, name)
17+
assert isinstance(exported, types.UnionType), (
18+
f"{name} must be a union alias, not a synthetic dataclass"
19+
)
20+
21+
22+
def test_permission_approval_union_loaders_deserialize_expected_variants():
23+
session = rpc._load_PermissionDecisionApproveForSessionApproval(
24+
{"kind": "commands", "commandIdentifiers": ["git status"]}
25+
)
26+
location = rpc._load_PermissionDecisionApproveForLocationApproval({"kind": "read"})
27+
28+
assert isinstance(session, rpc.PermissionDecisionApproveForSessionApprovalCommands)
29+
assert isinstance(location, rpc.PermissionDecisionApproveForLocationApprovalRead)

scripts/codegen/python.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,95 @@ function collapsePlaceholderPythonDataclasses(code: string, knownDefinitionNames
928928
return code.replace(/\n{3,}/g, "\n\n");
929929
}
930930

931+
function removeUnusedSyntheticPythonDataclasses(code: string, knownDefinitionNames: Set<string>): string {
932+
interface DataclassBlock {
933+
name: string;
934+
text: string;
935+
start: number;
936+
end: number;
937+
synthetic: boolean;
938+
}
939+
940+
const classBlockRe =
941+
/((?:^# (?:Experimental|Deprecated|Internal):[^\n]*\r?\n)*@dataclass(?:\([^\r\n]*\))?\r?\nclass\s+(\w+):[\s\S]*?)(?=^(?:# (?:Experimental|Deprecated|Internal):[^\n]*\r?\n)*@dataclass(?:\([^\r\n]*\))?\r?\nclass\s+\w|^class\s+\w|^def\s+\w|^[A-Z]\w+\s*=|\Z)/gm;
942+
const blocks: DataclassBlock[] = [...code.matchAll(classBlockRe)].map((match) => ({
943+
name: match[2],
944+
text: match[1],
945+
start: match.index ?? 0,
946+
end: (match.index ?? 0) + match[1].length,
947+
synthetic: !knownDefinitionNames.has(match[2].toLowerCase()),
948+
}));
949+
const syntheticBlocks = blocks.filter((block) => block.synthetic);
950+
if (syntheticBlocks.length === 0) return code;
951+
952+
let outsideSyntheticBlocks = "";
953+
let cursor = 0;
954+
for (const block of syntheticBlocks) {
955+
outsideSyntheticBlocks += code.slice(cursor, block.start);
956+
cursor = block.end;
957+
}
958+
outsideSyntheticBlocks += code.slice(cursor);
959+
960+
const syntheticNames = new Set(syntheticBlocks.map((block) => block.name));
961+
const dependencies = new Map<string, Set<string>>();
962+
const live = new Set<string>();
963+
964+
for (const block of syntheticBlocks) {
965+
const referenceRe = new RegExp(`\\b${escapeRegExp(block.name)}\\b`);
966+
if (referenceRe.test(outsideSyntheticBlocks)) {
967+
live.add(block.name);
968+
}
969+
970+
const blockDependencies = new Set<string>();
971+
for (const dependency of syntheticNames) {
972+
if (dependency === block.name) continue;
973+
const dependencyRe = new RegExp(`\\b${escapeRegExp(dependency)}\\b`);
974+
if (dependencyRe.test(block.text)) {
975+
blockDependencies.add(dependency);
976+
}
977+
}
978+
dependencies.set(block.name, blockDependencies);
979+
}
980+
981+
const worklist = [...live];
982+
while (worklist.length > 0) {
983+
const name = worklist.pop()!;
984+
for (const dependency of dependencies.get(name) ?? []) {
985+
if (live.has(dependency)) continue;
986+
live.add(dependency);
987+
worklist.push(dependency);
988+
}
989+
}
990+
991+
const blocksToRemove = new Set(syntheticBlocks.filter((block) => !live.has(block.name)).map((block) => block.name));
992+
if (blocksToRemove.size === 0) return code;
993+
994+
const appendSegment = (parts: string[], segment: string): void => {
995+
if (parts.length === 0 || segment.length === 0) {
996+
parts.push(segment);
997+
return;
998+
}
999+
const previous = parts[parts.length - 1];
1000+
const trailingNewlines = previous.match(/\n+$/)?.[0].length ?? 0;
1001+
const leadingNewlines = segment.match(/^\n+/)?.[0].length ?? 0;
1002+
if (trailingNewlines + leadingNewlines > 2) {
1003+
segment = "\n".repeat(Math.max(0, 2 - trailingNewlines)) + segment.slice(leadingNewlines);
1004+
}
1005+
parts.push(segment);
1006+
};
1007+
1008+
const parts: string[] = [];
1009+
cursor = 0;
1010+
for (const block of blocks) {
1011+
if (!blocksToRemove.has(block.name)) continue;
1012+
appendSegment(parts, code.slice(cursor, block.start));
1013+
cursor = block.end;
1014+
}
1015+
appendSegment(parts, code.slice(cursor));
1016+
1017+
return parts.join("");
1018+
}
1019+
9311020
/**
9321021
* Reorder Python class/enum definitions so forward references are resolved.
9331022
* Quicktype may emit classes in an order where a class references another
@@ -3261,6 +3350,10 @@ def _patch_model_capabilities(data: dict) -> dict:
32613350
finalCode = applyUnionRewritesToPython(finalCode, refBasedUnions);
32623351
finalCode = postProcessDiscriminatorDefaultsForPython(finalCode, refBasedUnions);
32633352
finalCode = unwrapRedundantPythonLambdas(finalCode);
3353+
finalCode = removeUnusedSyntheticPythonDataclasses(
3354+
finalCode,
3355+
new Set(Object.keys(allDefinitions).map((name) => name.toLowerCase()))
3356+
);
32643357

32653358
// Apply `_`-prefix to type names of internal RPC types so the leading-underscore
32663359
// Python convention signals "internal, no stability guarantees" to consumers.

0 commit comments

Comments
 (0)