From 0cec50119f803723f608f6498975799ab35abf52 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 5 Feb 2024 08:47:00 +0100 Subject: [PATCH] qapi: Require member documentation (with loophole) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The QAPI generator forces you to document your stuff. Except for command arguments, event data, and members of enum and object types: these the generator silently "documents" as "Not documented". We can't require proper documentation there without first fixing all the offenders. We've always had too many offenders to pull that off. Right now, we have more than 500. Worse, we seem to fix old ones no faster than we add new ones: in the past year, we fixed 22 ones, but added 26 new ones. To help arrest the backsliding, make missing documentation an error unless the command, type, or event is in listed in new pragma documentation-exceptions. List all the current offenders: 117 commands and types in qapi/, and 9 in qga/. Signed-off-by: Markus Armbruster Message-ID: <20240205074709.3613229-7-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé --- docs/devel/qapi-code-gen.rst | 5 + qapi/pragma.json | 119 ++++++++++++++++++ qga/qapi-schema.json | 13 +- scripts/qapi/parser.py | 7 +- scripts/qapi/source.py | 2 + .../qapi-schema/doc-bad-alternate-member.json | 2 + tests/qapi-schema/doc-good.json | 4 +- 7 files changed, 149 insertions(+), 3 deletions(-) diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst index 69c8a1e8bd7..756adc187e3 100644 --- a/docs/devel/qapi-code-gen.rst +++ b/docs/devel/qapi-code-gen.rst @@ -167,6 +167,7 @@ Syntax:: '*doc-required': BOOL, '*command-name-exceptions': [ STRING, ... ], '*command-returns-exceptions': [ STRING, ... ], + '*documentation-exceptions': [ STRING, ... ], '*member-name-exceptions': [ STRING, ... ] } } The pragma directive lets you control optional generator behavior. @@ -183,6 +184,10 @@ may contain ``"_"`` instead of ``"-"``. Default is none. Pragma 'command-returns-exceptions' takes a list of commands that may violate the rules on permitted return types. Default is none. +Pragma 'documentation-exceptions' takes a list of types, commands, and +events whose members / arguments need not be documented. Default is +none. + Pragma 'member-name-exceptions' takes a list of types whose member names may contain uppercase letters, and ``"_"`` instead of ``"-"``. Default is none. diff --git a/qapi/pragma.json b/qapi/pragma.json index 0aa4eeddd38..0fa64742b53 100644 --- a/qapi/pragma.json +++ b/qapi/pragma.json @@ -31,6 +31,125 @@ 'query-tpm-models', 'query-tpm-types', 'ringbuf-read' ], + # Types, commands, and events with undocumented members / arguments: + 'documentation-exceptions': [ + 'AbortWrapper', + 'AudiodevDriver', + 'BlkdebugEvent', + 'BlockDirtyBitmapAddWrapper', + 'BlockDirtyBitmapMergeWrapper', + 'BlockDirtyBitmapWrapper', + 'BlockExportOptions', + 'BlockStatsSpecific', + 'BlockdevBackupWrapper', + 'BlockdevDriver', + 'BlockdevQcow2Encryption', + 'BlockdevQcow2EncryptionFormat', + 'BlockdevQcowEncryption', + 'BlockdevSnapshotInternalWrapper', + 'BlockdevSnapshotSyncWrapper', + 'BlockdevSnapshotWrapper', + 'BlockdevVmdkAdapterType', + 'ChardevBackend', + 'ChardevBackendKind', + 'ChardevCommonWrapper', + 'ChardevDBusWrapper', + 'ChardevFileWrapper', + 'ChardevHostdevWrapper', + 'ChardevMuxWrapper', + 'ChardevQemuVDAgentWrapper', + 'ChardevRingbufWrapper', + 'ChardevSocketWrapper', + 'ChardevSpiceChannelWrapper', + 'ChardevSpicePortWrapper', + 'ChardevStdioWrapper', + 'ChardevUdpWrapper', + 'ChardevVCWrapper', + 'CpuS390Entitlement', + 'CpuS390Polarization', + 'CpuS390State', + 'CxlCorErrorType', + 'DisplayProtocol', + 'DriveBackupWrapper', + 'DummyBlockCoreForceArrays', + 'DummyForceArrays', + 'DummyVirtioForceArrays', + 'DumpGuestMemoryCapability', + 'GrabToggleKeys', + 'GuestPanicInformationHyperV', + 'HotKeyMod', + 'HvBalloonDeviceInfoWrapper', + 'ImageInfoSpecific', + 'ImageInfoSpecificFileWrapper', + 'ImageInfoSpecificKind', + 'ImageInfoSpecificLUKSWrapper', + 'ImageInfoSpecificQCow2Wrapper', + 'ImageInfoSpecificRbdWrapper', + 'ImageInfoSpecificVmdkWrapper', + 'InetSocketAddressWrapper', + 'InputAxis', + 'InputBtnEventWrapper', + 'InputButton', + 'InputKeyEventWrapper', + 'InputMoveEventWrapper', + 'InputMultiTouchEvent', + 'InputMultiTouchEventWrapper', + 'InputMultiTouchType', + 'IntWrapper', + 'IscsiHeaderDigest', + 'IscsiTransport', + 'JSONType', + 'KeyValue', + 'KeyValueKind', + 'MemoryDeviceInfo', + 'MemoryDeviceInfoKind', + 'MigrateSetParameters', + 'MigrationAddress', + 'NetClientDriver', + 'NumaOptions', + 'ObjectType', + 'PCDIMMDeviceInfoWrapper', + 'PciMemoryRegion', + 'QCryptoAkCipherKeyType', + 'QCryptoAkCipherOptions', + 'QCryptodevBackendServiceType', + 'QKeyCode', + 'QKeyCodeWrapper', + 'Qcow2OverlapCheckFlags', + 'RbdAuthMode', + 'RbdEncryptionCreateOptions', + 'RbdImageEncryptionFormat', + 'SgxEPCDeviceInfoWrapper', + 'SocketAddressLegacy', + 'SshHostKeyCheck', + 'StatsFilter', + 'StatsValue', + 'String', + 'StringWrapper', + 'SysEmuTarget', + 'TPMEmulatorOptionsWrapper', + 'TPMPassthroughOptionsWrapper', + 'ThrottleGroupProperties', + 'TransactionAction', + 'UnixSocketAddressWrapper', + 'VirtioMEMDeviceInfoWrapper', + 'VirtioPMEMDeviceInfoWrapper', + 'VncPrimaryAuth', + 'VncVencryptSubAuth', + 'VsockSocketAddressWrapper', + 'X86CPURegister32', + 'XDbgBlockGraph', + 'YankInstance', + 'YankInstanceType', + 'blockdev-reopen', + 'query-cpu-model-baseline', + 'query-cpu-model-comparison', + 'query-cpu-model-expansion', + 'query-rocker', + 'query-rocker-ports', + 'query-stats-schemas', + 'watchdog-set-action', + 'yank' ], # Externally visible types whose member names may use uppercase 'member-name-exceptions': [ # visible in: 'ACPISlotType', # query-acpi-ospm-status diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index 50b0a558c7b..b9501c8c811 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -33,7 +33,18 @@ 'guest-get-time', 'guest-set-vcpus', 'guest-sync', - 'guest-sync-delimited' ] } } + 'guest-sync-delimited' ], + # Types and commands with undocumented members: + 'documentation-exceptions': [ + 'GuestCpuStats', + 'GuestCpuStatsType', + 'GuestDeviceId', + 'GuestDeviceType', + 'GuestDiskSmart', + 'GuestDiskStatsInfo', + 'GuestNVMeSmart', + 'guest-set-memory-blocks', + 'guest-set-vcpus' ] } } ## # @guest-sync-delimited: diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index 48cd55a38cc..88221b3c640 100644 --- a/scripts/qapi/parser.py +++ b/scripts/qapi/parser.py @@ -238,6 +238,8 @@ class QAPISchemaParser: pragma.command_name_exceptions = check_list_str(name, value) elif name == 'command-returns-exceptions': pragma.command_returns_exceptions = check_list_str(name, value) + elif name == 'documentation-exceptions': + pragma.documentation_exceptions = check_list_str(name, value) elif name == 'member-name-exceptions': pragma.member_name_exceptions = check_list_str(name, value) else: @@ -739,7 +741,10 @@ class QAPIDoc: def connect_member(self, member: 'QAPISchemaMember') -> None: if member.name not in self.args: - # Undocumented TODO outlaw + if self.symbol not in member.info.pragma.documentation_exceptions: + raise QAPISemError(member.info, + "%s '%s' lacks documentation" + % (member.role, member.name)) self.args[member.name] = QAPIDoc.ArgSection(self._parser, member.name) self.args[member.name].connect(member) diff --git a/scripts/qapi/source.py b/scripts/qapi/source.py index 04193cc9643..7b379fdc925 100644 --- a/scripts/qapi/source.py +++ b/scripts/qapi/source.py @@ -24,6 +24,8 @@ class QAPISchemaPragma: self.command_name_exceptions: List[str] = [] # Commands allowed to return a non-dictionary self.command_returns_exceptions: List[str] = [] + # Types, commands, and events with undocumented members + self.documentation_exceptions: List[str] = [] # Types whose member names may violate case conventions self.member_name_exceptions: List[str] = [] diff --git a/tests/qapi-schema/doc-bad-alternate-member.json b/tests/qapi-schema/doc-bad-alternate-member.json index fa4143da4c4..37593b6698c 100644 --- a/tests/qapi-schema/doc-bad-alternate-member.json +++ b/tests/qapi-schema/doc-bad-alternate-member.json @@ -2,6 +2,8 @@ ## # @AorB: +# @a: a +# @b: b # @aa: a # @bb: b ## diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json index 976f9e1aaaf..24a84fe6d7a 100644 --- a/tests/qapi-schema/doc-good.json +++ b/tests/qapi-schema/doc-good.json @@ -3,7 +3,9 @@ # # Positive QAPI doc comment tests -{ 'pragma': { 'doc-required': true } } +{ 'pragma': { + 'doc-required': true, + 'documentation-exceptions': [ 'Enum', 'Variant1', 'Alternate', 'cmd' ] } } ## # = Section -- 2.39.5