diff --git a/hacking/metadata-tool.py b/hacking/metadata-tool.py index 418feee5784..1069c500074 100755 --- a/hacking/metadata-tool.py +++ b/hacking/metadata-tool.py @@ -205,7 +205,8 @@ def write_metadata(filename, new_metadata, version=None, overwrite=False): module_data = f.read() try: - current_metadata, start_line, start_col, end_line, end_col, targets = extract_metadata(module_data) + current_metadata, start_line, start_col, end_line, end_col, targets = \ + extract_metadata(module_data=module_data, offsets=True) except SyntaxError: if filename.endswith('.py'): raise @@ -257,7 +258,7 @@ def return_metadata(plugins): if name not in metadata or metadata[name] is not None: with open(filename, 'rb') as f: module_data = f.read() - metadata[name] = extract_metadata(module_data)[0] + metadata[name] = extract_metadata(module_data=module_data, offsets=True)[0] return metadata @@ -408,7 +409,7 @@ def upgrade_metadata(version=None): # For each plugin, read the existing metadata with open(filename, 'rb') as f: module_data = f.read() - metadata = extract_metadata(module_data)[0] + metadata = extract_metadata(module_data=module_data, offsets=True)[0] # If the metadata isn't the requested version, convert it to the new # version diff --git a/lib/ansible/cli/doc.py b/lib/ansible/cli/doc.py index 5e7b39b9d25..7f9ba3f2f63 100644 --- a/lib/ansible/cli/doc.py +++ b/lib/ansible/cli/doc.py @@ -224,6 +224,7 @@ class DocCLI(CLI): if os.path.isdir(filename): continue + doc = None try: doc, plainexamples, returndocs, metadata = plugin_docs.get_docstring(filename) except: diff --git a/lib/ansible/parsing/metadata.py b/lib/ansible/parsing/metadata.py index 8b2f85f6866..f22570612d2 100644 --- a/lib/ansible/parsing/metadata.py +++ b/lib/ansible/parsing/metadata.py @@ -20,6 +20,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type import ast +import sys import yaml @@ -146,32 +147,53 @@ def _seek_end_of_string(module_data, start_line, start_col, next_node_line, next raise NotImplementedError('Finding end of string not yet implemented') -def extract_metadata(module_data): +def extract_metadata(module_ast=None, module_data=None, offsets=False): """Extract the metadata from a module - :arg module_data: Byte string containing a module's code + :kwarg module_ast: ast representation of the module. At least one of this + or ``module_data`` must be given. If the code calling + :func:`extract_metadata` has already parsed the module_data into an ast, + giving the ast here will save reparsing it. + :kwarg module_data: Byte string containing a module's code. At least one + of this or ``module_ast`` must be given. + :kwarg offsets: If set to True, offests into the source code will be + returned. This requires that ``module_data`` be set. :returns: a tuple of metadata (a dict), line the metadata starts on, column the metadata starts on, line the metadata ends on, column the metadata ends on, and the names the metadata is assigned to. One of - the names the metadata is assigned to will be ANSIBLE_METADATA If no - metadata is found, the tuple will be (None, -1, -1, -1, -1, None) + the names the metadata is assigned to will be ANSIBLE_METADATA. If no + metadata is found, the tuple will be (None, -1, -1, -1, -1, None). + If ``offsets`` is False then the tuple will consist of + (metadata, -1, -1, -1, -1, None). + :raises ansible.parsing.metadata.ParseError: if ``module_data`` does not parse + :raises SyntaxError: if ``module_data`` is needed but does not parse correctly """ + if offsets and module_data is None: + raise TypeError('If offsets is True then module_data must also be given') + + if module_ast is None and module_data is None: + raise TypeError('One of module_ast or module_data must be given') + metadata = None start_line = -1 start_col = -1 end_line = -1 end_col = -1 targets = None - mod_ast_tree = ast.parse(module_data) - for root_idx, child in enumerate(mod_ast_tree.body): + if module_ast is None: + module_ast = ast.parse(module_data) + + for root_idx, child in reversed(list(enumerate(module_ast.body))): if isinstance(child, ast.Assign): for target in child.targets: if target.id == 'ANSIBLE_METADATA': metadata = ast.literal_eval(child.value) + if not offsets: + continue try: # Determine where the next node starts - next_node = mod_ast_tree.body[root_idx + 1] + next_node = module_ast.body[root_idx + 1] next_lineno = next_node.lineno next_col_offset = next_node.col_offset except IndexError: @@ -202,10 +224,7 @@ def extract_metadata(module_data): next_lineno, next_col_offset) else: - # Example: - # ANSIBLE_METADATA = 'junk' - # ANSIBLE_METADATA = { [..the real metadata..] } - continue + raise ParseError('Ansible plugin metadata must be a dict') # Do these after the if-else so we don't pollute them in # case this was a false positive diff --git a/lib/ansible/utils/plugin_docs.py b/lib/ansible/utils/plugin_docs.py index b62505f0b32..e93d08856ae 100644 --- a/lib/ansible/utils/plugin_docs.py +++ b/lib/ansible/utils/plugin_docs.py @@ -155,7 +155,7 @@ def get_docstring(filename, verbose=False): data[varkey] = child.value.s display.debug('assigned :%s' % varkey) - data['metadata'] = extract_metadata(b_module_data)[0] + data['metadata'] = extract_metadata(module_ast=M)[0] # add fragments to documentation if data['doc']: add_fragments(data['doc'], filename) @@ -165,7 +165,7 @@ def get_docstring(filename, verbose=False): for x in ('version', 'metadata_version'): if x in data['metadata']: del data['metadata'][x] - except: + except Exception as e: display.error("unable to parse %s" % filename) if verbose is True: display.display("unable to parse %s" % filename) diff --git a/test/units/parsing/test_metadata.py b/test/units/parsing/test_metadata.py index ec05d6ff447..72adc845c1e 100644 --- a/test/units/parsing/test_metadata.py +++ b/test/units/parsing/test_metadata.py @@ -20,10 +20,13 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -from ansible.parsing import metadata as md +import ast import pytest +from ansible.parsing import metadata as md + + LICENSE = b"""# some license text boilerplate # That we have at the top of files """ @@ -43,8 +46,17 @@ ANSIBLE_METADATA = {'metadata_version': '1.0', 'supported_by': 'core'} """ -STRING_STD_METADATA = b""" -ANSIBLE_METADATA = ''' +TEXT_STD_METADATA = b""" +ANSIBLE_METADATA = u''' +metadata_version: '1.0' +status: + - 'stableinterface' +supported_by: 'core' +''' +""" + +BYTES_STD_METADATA = b""" +ANSIBLE_METADATA = b''' metadata_version: '1.0' status: - 'stableinterface' @@ -161,31 +173,67 @@ METADATA_EXAMPLES = ( (HASH_SYMBOL_METADATA, 1, 0, 4, 42, ['ANSIBLE_METADATA'])), # Standard import with a junk ANSIBLE_METADATA as well - (LICENSE + FUTURE_IMPORTS + b"\nANSIBLE_METADAtA = 'junk'\n" + HASH_COMBO_METADATA + REGULAR_IMPORTS, + (LICENSE + FUTURE_IMPORTS + b"\nANSIBLE_METADATA = 10\n" + HASH_COMBO_METADATA + REGULAR_IMPORTS, (HASH_SYMBOL_METADATA, 7, 0, 10, 42, ['ANSIBLE_METADATA'])), ) # FIXME: String/yaml metadata is not implemented yet. Need more test cases once it is implemented STRING_METADATA_EXAMPLES = ( # Standard import - (LICENSE + FUTURE_IMPORTS + STRING_STD_METADATA + REGULAR_IMPORTS, + (LICENSE + FUTURE_IMPORTS + TEXT_STD_METADATA + REGULAR_IMPORTS, (METADATA, 5, 0, 10, 3, ['ANSIBLE_METADATA'])), # Metadata at end of file - (LICENSE + FUTURE_IMPORTS + REGULAR_IMPORTS + STRING_STD_METADATA.rstrip(), + (LICENSE + FUTURE_IMPORTS + REGULAR_IMPORTS + TEXT_STD_METADATA.rstrip(), (METADATA, 8, 0, 13, 3, ['ANSIBLE_METADATA'])), # Metadata at beginning of file - (STRING_STD_METADATA + LICENSE + REGULAR_IMPORTS, + (TEXT_STD_METADATA + LICENSE + REGULAR_IMPORTS, + (METADATA, 1, 0, 6, 3, ['ANSIBLE_METADATA'])), + + # Standard import + (LICENSE + FUTURE_IMPORTS + BYTES_STD_METADATA + REGULAR_IMPORTS, + (METADATA, 5, 0, 10, 3, ['ANSIBLE_METADATA'])), + # Metadata at end of file + (LICENSE + FUTURE_IMPORTS + REGULAR_IMPORTS + BYTES_STD_METADATA.rstrip(), + (METADATA, 8, 0, 13, 3, ['ANSIBLE_METADATA'])), + # Metadata at beginning of file + (BYTES_STD_METADATA + LICENSE + REGULAR_IMPORTS, (METADATA, 1, 0, 6, 3, ['ANSIBLE_METADATA'])), ) @pytest.mark.parametrize("code, expected", METADATA_EXAMPLES) -def test_extract_metadata(code, expected): - assert md.extract_metadata(code) == expected +def test_dict_metadata(code, expected): + assert md.extract_metadata(module_data=code, offsets=True) == expected @pytest.mark.parametrize("code, expected", STRING_METADATA_EXAMPLES) -def test_extract_string_metadata(code, expected): +def test_string_metadata(code, expected): # FIXME: String/yaml metadata is not implemented yet. with pytest.raises(NotImplementedError): - assert md.extract_metadata(code) == expected + assert md.extract_metadata(module_data=code, offsets=True) == expected + + +def test_required_params(): + with pytest.raises(TypeError, message='One of module_ast or module_data must be given'): + assert md.extract_metadata() + + +def test_module_data_param_given_with_offset(): + with pytest.raises(TypeError, message='If offsets is True then module_data must also be given'): + assert md.extract_metadata(module_ast='something', offsets=True) + + +def test_invalid_dict_metadata(): + with pytest.raises(SyntaxError): + assert md.extract_metadata(module_data=LICENSE + FUTURE_IMPORTS + b'ANSIBLE_METADATA={"metadata_version": "1.0",\n' + REGULAR_IMPORTS) + + with pytest.raises(md.ParseError, message='Unable to find the end of dictionary'): + assert md.extract_metadata(module_ast=ast.parse(LICENSE + FUTURE_IMPORTS + b'ANSIBLE_METADATA={"metadata_version": "1.0"}\n' + REGULAR_IMPORTS), + module_data=LICENSE + FUTURE_IMPORTS + b'ANSIBLE_METADATA={"metadata_version": "1.0",\n' + REGULAR_IMPORTS, + offsets=True) + + +def test_multiple_statements_limitation(): + with pytest.raises(md.ParseError, message='Multiple statements per line confuses the module metadata parser.'): + assert md.extract_metadata(module_data=LICENSE + FUTURE_IMPORTS + b'ANSIBLE_METADATA={"metadata_version": "1.0"}; a=b\n' + REGULAR_IMPORTS, + offsets=True)