Render elements in module doc and sanity test for sub-options (#59244)

* Render elements in module doc and sanity test for suboptions

*  Add support to render module elements value in ansible-doc output
   module html
*  Add validate-module sanity test of sunoptions.

* Add current validate module failures to ignore list

* Fix CI failure

* fix rebase conflict

* Fix CI issues

* Fix review comments

* Add validate-modules failure in ignore list
pull/60312/head
Ganesh Nalawade 5 years ago committed by GitHub
parent 5d7cc993dd
commit 127bd67f6e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -133,6 +133,8 @@ Errors
336 argument in argument_spec is not a valid python identifier 336 argument in argument_spec is not a valid python identifier
337 Type value is defined in ``argument_spec`` but documentation doesn't specify a type 337 Type value is defined in ``argument_spec`` but documentation doesn't specify a type
338 documentation doesn't specify a type but argument in ``argument_spec`` use default type (``str``) 338 documentation doesn't specify a type but argument in ``argument_spec`` use default type (``str``)
339 Value for "elements" is valid only when value of "type" is ``list``
340 argument in argument_spec has sub-options but documentation does not define sub-options
.. ..
--------- ------------------- --------- -------------------
**4xx** **Syntax** **4xx** **Syntax**

@ -120,6 +120,7 @@ Parameters
<b>@{ key }@</b> <b>@{ key }@</b>
<div style="font-size: small"> <div style="font-size: small">
<span style="color: purple">@{ value.type | documented_type }@</span> <span style="color: purple">@{ value.type | documented_type }@</span>
{% if value.get('elements') %} / <span style="color: purple">elements=@{ value.elements | documented_type }@</span>{% endif %}
{% if value.get('required', False) %} / <span style="color: red">required</span>{% endif %} {% if value.get('required', False) %} / <span style="color: red">required</span>{% endif %}
</div> </div>
{% if value.version_added %}<div style="font-style: italic; font-size: small; color: darkgreen">added in @{value.version_added}@</div>{% endif %} {% if value.version_added %}<div style="font-style: italic; font-size: small; color: darkgreen">added in @{value.version_added}@</div>{% endif %}

File diff suppressed because it is too large Load Diff

@ -1144,15 +1144,19 @@ class ModuleValidator(Validator):
self._validate_argument_spec(docs, spec, kwargs) self._validate_argument_spec(docs, spec, kwargs)
def _validate_argument_spec(self, docs, spec, kwargs): def _validate_argument_spec(self, docs, spec, kwargs, context=None):
if not self.analyze_arg_spec: if not self.analyze_arg_spec:
return return
if docs is None: if docs is None:
docs = {} docs = {}
if context is None:
context = []
try: try:
add_fragments(docs, self.object_path, fragment_loader=fragment_loader) if not context:
add_fragments(docs, self.object_path, fragment_loader=fragment_loader)
except Exception: except Exception:
# Cannot merge fragments # Cannot merge fragments
return return
@ -1165,10 +1169,14 @@ class ModuleValidator(Validator):
deprecated_args_from_argspec = set() deprecated_args_from_argspec = set()
for arg, data in spec.items(): for arg, data in spec.items():
if not isinstance(data, dict): if not isinstance(data, dict):
msg = "Argument '%s' in argument_spec" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " must be a dictionary/hash when used"
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=331, code=331,
msg="Argument '%s' in argument_spec must be a dictionary/hash when used" % arg, msg=msg,
) )
continue continue
if not data.get('removed_in_version', None): if not data.get('removed_in_version', None):
@ -1191,11 +1199,15 @@ class ModuleValidator(Validator):
provider_args.update(provider_data.get('aliases', [])) provider_args.update(provider_data.get('aliases', []))
if data.get('required') and data.get('default', object) != object: if data.get('required') and data.get('default', object) != object:
msg = "Argument '%s' in argument_spec" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " is marked as required but specifies a default. Arguments with a" \
" default should not be marked as required"
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=317, code=317,
msg=("Argument '%s' in argument_spec is marked as required " msg=msg
"but specifies a default. Arguments with a default should not be marked as required" % arg)
) )
if arg in provider_args: if arg in provider_args:
@ -1209,18 +1221,35 @@ class ModuleValidator(Validator):
else: else:
_type_checker = module._CHECK_ARGUMENT_TYPES_DISPATCHER.get(_type) _type_checker = module._CHECK_ARGUMENT_TYPES_DISPATCHER.get(_type)
# TODO: needs to recursively traverse suboptions _elements = data.get('elements')
if _elements:
if not callable(_elements):
module._CHECK_ARGUMENT_TYPES_DISPATCHER.get(_elements)
if _type != 'list':
msg = "Argument '%s' in argument_spec" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " defines elements as %s but it is valid only when value of parameter type is list" % _elements
self.reporter.error(
path=self.object_path,
code=339,
msg=msg
)
arg_default = None arg_default = None
if 'default' in data and not is_empty(data['default']): if 'default' in data and not is_empty(data['default']):
try: try:
with CaptureStd(): with CaptureStd():
arg_default = _type_checker(data['default']) arg_default = _type_checker(data['default'])
except (Exception, SystemExit): except (Exception, SystemExit):
msg = "Argument '%s' in argument_spec" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " defines default as (%r) but this is incompatible with parameter type %r" % (data['default'], _type)
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=329, code=329,
msg=("Argument '%s' in argument_spec defines default as (%r) " msg=msg
"but this is incompatible with parameter type %r" % (arg, data['default'], _type))
) )
continue continue
elif data.get('default') is None and _type == 'bool' and 'options' not in data: elif data.get('default') is None and _type == 'bool' and 'options' not in data:
@ -1235,57 +1264,73 @@ class ModuleValidator(Validator):
elif doc_options_arg.get('default') is None and _type == 'bool' and 'suboptions' not in doc_options_arg: elif doc_options_arg.get('default') is None and _type == 'bool' and 'suboptions' not in doc_options_arg:
doc_default = False doc_default = False
except (Exception, SystemExit): except (Exception, SystemExit):
msg = "Argument '%s' in documentation" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " defines default as (%r) but this is incompatible with parameter type %r" % (doc_options_arg.get('default'), _type)
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=327, code=327,
msg=("Argument '%s' in documentation defines default as (%r) " msg=msg
"but this is incompatible with parameter type %r" % (arg, doc_options_arg.get('default'), _type))
) )
continue continue
if arg_default != doc_default: if arg_default != doc_default:
msg = "Argument '%s' in argument_spec" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " defines default as (%r) but documentation defines default as (%r)" % (arg_default, doc_default)
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=324, code=324,
msg=("Argument '%s' in argument_spec defines default as (%r) " msg=msg
"but documentation defines default as (%r)" % (arg, arg_default, doc_default))
) )
# TODO: needs to recursively traverse suboptions
doc_type = docs.get('options', {}).get(arg, {}).get('type') doc_type = docs.get('options', {}).get(arg, {}).get('type')
if 'type' in data and data['type'] is not None: if 'type' in data and data['type'] is not None:
if doc_type is None: if doc_type is None:
if not arg.startswith('_'): # hidden parameter, for example _raw_params if not arg.startswith('_'): # hidden parameter, for example _raw_params
msg = "Argument '%s' in argument_spec" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " defines type as %r but documentation doesn't define type" % (data['type'])
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=337, code=337,
msg="Argument '%s' in argument_spec defines type as %r " msg=msg
"but documentation doesn't define type" % (arg, data['type'])
) )
elif data['type'] != doc_type: elif data['type'] != doc_type:
msg = "Argument '%s' in argument_spec" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " defines type as %r but documentation defines type as %r" % (data['type'], doc_type)
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=325, code=325,
msg="Argument '%s' in argument_spec defines type as %r " msg=msg
"but documentation defines type as %r" % (arg, data['type'], doc_type)
) )
else: else:
if doc_type is None: if doc_type is None:
msg = "Argument '%s' in argument_spec" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " uses default type ('str') but documentation doesn't define type"
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=338, code=338,
msg="Argument '%s' in argument_spec uses default type ('str') " msg=msg
"but documentation doesn't define type" % (arg)
) )
elif doc_type != 'str': elif doc_type != 'str':
msg = "Argument '%s' in argument_spec" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += "implies type as 'str' but documentation defines as %r" % doc_type
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=335, code=335,
msg="Argument '%s' in argument_spec implies type as 'str' " msg=msg
"but documentation defines as %r" % (arg, doc_type)
) )
# TODO: needs to recursively traverse suboptions
doc_choices = [] doc_choices = []
try: try:
for choice in docs.get('options', {}).get(arg, {}).get('choices', []): for choice in docs.get('options', {}).get(arg, {}).get('choices', []):
@ -1293,11 +1338,14 @@ class ModuleValidator(Validator):
with CaptureStd(): with CaptureStd():
doc_choices.append(_type_checker(choice)) doc_choices.append(_type_checker(choice))
except (Exception, SystemExit): except (Exception, SystemExit):
msg = "Argument '%s' in documentation" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " defines choices as (%r) but this is incompatible with argument type %r" % (choice, _type)
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=328, code=328,
msg=("Argument '%s' in documentation defines choices as (%r) " msg=msg
"but this is incompatible with argument type %r" % (arg, choice, _type))
) )
raise StopIteration() raise StopIteration()
except StopIteration: except StopIteration:
@ -1310,30 +1358,55 @@ class ModuleValidator(Validator):
with CaptureStd(): with CaptureStd():
arg_choices.append(_type_checker(choice)) arg_choices.append(_type_checker(choice))
except (Exception, SystemExit): except (Exception, SystemExit):
msg = "Argument '%s' in argument_spec" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " defines choices as (%r) but this is incompatible with argument type %r" % (choice, _type)
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=330, code=330,
msg=("Argument '%s' in argument_spec defines choices as (%r) " msg=msg
"but this is incompatible with argument type %r" % (arg, choice, _type))
) )
raise StopIteration() raise StopIteration()
except StopIteration: except StopIteration:
continue continue
if not compare_unordered_lists(arg_choices, doc_choices): if not compare_unordered_lists(arg_choices, doc_choices):
msg = "Argument '%s' in argument_spec" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " defines choices as (%r) but documentation defines choices as (%r)" % (arg_choices, doc_choices)
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=326, code=326,
msg=("Argument '%s' in argument_spec defines choices as (%r) " msg=msg
"but documentation defines choices as (%r)" % (arg, arg_choices, doc_choices))
) )
spec_suboptions = data.get('options')
doc_suboptions = docs.get('options', {}).get(arg, {}).get('suboptions', {})
if spec_suboptions:
if not doc_suboptions:
msg = "Argument '%s' in argument_spec" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " has sub-options but documentation does not define it"
self.reporter.error(
path=self.object_path,
code=340,
msg=msg
)
self._validate_argument_spec({'options': doc_suboptions}, spec_suboptions, kwargs, context=context + [arg])
for arg in args_from_argspec: for arg in args_from_argspec:
if not str(arg).isidentifier(): if not str(arg).isidentifier():
msg = "Argument '%s' in argument_spec" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " is not a valid python identifier"
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=336, code=336,
msg="Argument '%s' is not a valid python identifier" % arg msg=msg
) )
if docs: if docs:
@ -1358,22 +1431,28 @@ class ModuleValidator(Validator):
# Provider args are being removed from network module top level # Provider args are being removed from network module top level
# So they are likely not documented on purpose # So they are likely not documented on purpose
continue continue
msg = "Argument '%s'" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " is listed in the argument_spec, but not documented in the module documentation"
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=322, code=322,
msg="Argument '%s' is listed in the argument_spec, " msg=msg
"but not documented in the module documentation" % arg
) )
for arg in docs_missing_from_args: for arg in docs_missing_from_args:
# args_from_docs contains argument not in the argument_spec # args_from_docs contains argument not in the argument_spec
if kwargs.get('add_file_common_args', False) and arg in file_common_arguments: if kwargs.get('add_file_common_args', False) and arg in file_common_arguments:
# add_file_common_args is handled in AnsibleModule, and not exposed earlier # add_file_common_args is handled in AnsibleModule, and not exposed earlier
continue continue
msg = "Argument '%s'" % arg
if context:
msg += " found in %s" % " -> ".join(context)
msg += " is listed in DOCUMENTATION.options, but not accepted by the module argument_spec"
self.reporter.error( self.reporter.error(
path=self.object_path, path=self.object_path,
code=323, code=323,
msg="Argument '%s' is listed in DOCUMENTATION.options, " msg=msg
"but not accepted by the module argument_spec" % arg
) )
def _check_for_new_args(self, doc, metadata): def _check_for_new_args(self, doc, metadata):

@ -84,6 +84,8 @@ suboption_schema = Schema(
'default': Any(None, float, int, bool, list, dict, *string_types), 'default': Any(None, float, int, bool, list, dict, *string_types),
# Note: Types are strings, not literal bools, such as True or False # Note: Types are strings, not literal bools, such as True or False
'type': Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'sid', 'str'), 'type': Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'sid', 'str'),
# in case of type='list' elements define type of individual item in list
'elements': Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'sid', 'str'),
# Recursive suboptions # Recursive suboptions
'suboptions': Any(None, *list({str_type: Self} for str_type in string_types)), 'suboptions': Any(None, *list({str_type: Self} for str_type in string_types)),
}, },
@ -105,6 +107,8 @@ option_schema = Schema(
'suboptions': Any(None, *list_dict_suboption_schema), 'suboptions': Any(None, *list_dict_suboption_schema),
# Note: Types are strings, not literal bools, such as True or False # Note: Types are strings, not literal bools, such as True or False
'type': Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'sid', 'str'), 'type': Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'sid', 'str'),
# in case of type='list' elements define type of individual item in list
'elements': Any(None, 'bits', 'bool', 'bytes', 'dict', 'float', 'int', 'json', 'jsonarg', 'list', 'path', 'raw', 'sid', 'str'),
}, },
extra=PREVENT_EXTRA extra=PREVENT_EXTRA
) )

Loading…
Cancel
Save