diff --git a/api/dev/lint_response_contracts.py b/api/dev/lint_response_contracts.py index 77a2d2dc818..6cfc1c6b446 100644 --- a/api/dev/lint_response_contracts.py +++ b/api/dev/lint_response_contracts.py @@ -595,9 +595,15 @@ def line_has_ignore_marker(line: str) -> bool: return any(ignore_marker in normalized for ignore_marker in IGNORE_COMMENT_MARKERS) +def node_ignore_scan_end_lineno(node: ast.ClassDef | MethodNode) -> int: + if isinstance(node, ast.ClassDef): + return node.lineno + return node.end_lineno or node.lineno + + def node_has_ignore_comment(lines: Sequence[str], node: ast.ClassDef | MethodNode) -> bool: start = node_start_lineno(node) - end = node.end_lineno or node.lineno + end = node_ignore_scan_end_lineno(node) if any(line_has_ignore_marker(line) for line in lines[start - 1 : end]): return True diff --git a/api/tests/unit_tests/commands/test_lint_response_contracts.py b/api/tests/unit_tests/commands/test_lint_response_contracts.py index 68c4cbf966e..17f3156d127 100644 --- a/api/tests/unit_tests/commands/test_lint_response_contracts.py +++ b/api/tests/unit_tests/commands/test_lint_response_contracts.py @@ -168,6 +168,29 @@ class RegularApi(Resource): assert checks[0].classification == "valid" +def test_method_ignore_comment_does_not_skip_sibling_methods(tmp_path: Path): + checks = _checks_for_source( + tmp_path, + """ +@ns.route("/mixed") +class MixedApi(Resource): + # response-contract:ignore binary response + @ns.response(200, "Binary file") + def get(self): + return send_file(path) + + @ns.response(200, "OK", ns.models[RegularResponse.__name__]) + def post(self): + return dump_response(RegularResponse, {}) +""", + ) + + assert len(checks) == 1 + assert checks[0].class_name == "MixedApi" + assert checks[0].method == "post" + assert checks[0].classification == "valid" + + def test_main_is_report_only_by_default_for_mismatches(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): module = _load_lint_response_contracts_module() controller_path = tmp_path / "controllers" / "sample.py" diff --git a/packages/contracts/openapi-ts.api.config.ts b/packages/contracts/openapi-ts.api.config.ts index 1adbf4fda8e..99e1f79ee2b 100644 --- a/packages/contracts/openapi-ts.api.config.ts +++ b/packages/contracts/openapi-ts.api.config.ts @@ -63,15 +63,6 @@ const operationMethods = new Set(['delete', 'get', 'patch', 'post', 'put']) const pydanticDecimalStringPattern = '^(?!^[-+.]*$)[+-]?0*\\d*\\.?\\d*$' const codegenSafeDecimalStringPattern = '^(?![-+.]*$)[+-]?0*\\d*\\.?\\d*$' -const opaqueJsonContent = (): Record => ({ - 'application/json': { - schema: { - additionalProperties: true, - type: 'object', - }, - }, -}) - const apiSpecs: ApiSpec[] = [ { filename: 'console-openapi.json', name: 'console' }, { filename: 'web-openapi.json', name: 'web' }, @@ -201,42 +192,49 @@ const addOperationIds = (document: SwaggerDocument) => { } } -const isOpaqueContractResponse = (response: OpenApiResponse) => { - const content = response.content - if (!isObject(content)) - return false - - return Object.entries(content).some(([mediaType, media]) => { - if (!isObject(media)) - return false - - return (mediaType === 'application/json' || mediaType === 'text/event-stream') && !('schema' in media) - }) -} - -const hasOpaqueContractSuccessResponse = (operation: SwaggerOperation) => { - return Object.entries(operation.responses ?? {}).some(([status, response]) => { - return /^2\d\d$/.test(status) && isObject(response) && isOpaqueContractResponse(response) - }) -} - const normalizeOpaqueContractResponses = (document: SwaggerDocument) => { - // Some backend endpoints has no schema (e.g. external) and will trap heyapi here - // So we forge an opaque schema here + // This runs before contract filtering. Flask-RESTX often emits plain success responses + // without a body schema; give those routes an opaque output so they stay in oRPC. for (const pathItem of Object.values(document.paths ?? {})) { for (const [method, operation] of Object.entries(pathItem)) { if (!operationMethods.has(method) || !isObject(operation)) continue const swaggerOperation = operation as SwaggerOperation - if (!hasOpaqueContractSuccessResponse(swaggerOperation)) - continue + for (const [status, response] of Object.entries(swaggerOperation.responses ?? {})) { + // Ignore non-2xx or 204 or those w/o a response field + if (!/^2\d\d$/.test(status) || status === '204' || !isObject(response)) + continue - Object.values(swaggerOperation.responses ?? {}) - .filter(response => isObject(response) && isOpaqueContractResponse(response)) - .forEach((response) => { - response.content = opaqueJsonContent() - }) + const content = response.content + if (!isObject(content) || Object.keys(content).length === 0) { + // No response specification, fill a dummy opaque resp + response.content = { + 'application/json': { + schema: { + additionalProperties: true, + type: 'object', + }, + }, + } + continue + } + + for (const [mediaType, media] of Object.entries(content)) { + if (mediaType !== 'application/json' && mediaType !== 'text/event-stream') + continue + if (!isObject(media) || isObject(media.schema)) + continue + + // JSON/SSE media without a schema traps heyapi. Patch only that media entry so + // sibling binary media keeps heyapi's Blob | File inference. + // Still a dummy opaque resp + media.schema = { + additionalProperties: true, + type: 'object', + } + } + } } } }