From 6af1b6049e3728d12adef1951b89c6e0279448c8 Mon Sep 17 00:00:00 2001 From: akshatkarani Date: Sat, 13 Oct 2018 16:52:45 +0530 Subject: [PATCH] Setting.py: Add SourcePosition to Setting.origin This adds SourcePosition to Setting.origin which helps in identifying the starting line number of a setting in .coafile. Related to https://github.com/coala/coala/issues/5044 --- coalib/parsing/ConfParser.py | 9 ++++-- coalib/results/SourcePosition.py | 9 ++++++ coalib/settings/ConfigurationGathering.py | 3 +- coalib/settings/Setting.py | 24 +++++++++++++-- tests/parsing/ConfParserTest.py | 28 +++++++++++++++++ tests/results/SourcePositionTest.py | 5 +++ tests/settings/SettingTest.py | 37 +++++++++++++++++++++++ 7 files changed, 109 insertions(+), 6 deletions(-) diff --git a/coalib/parsing/ConfParser.py b/coalib/parsing/ConfParser.py index 4e502e8afe..69790af5b6 100644 --- a/coalib/parsing/ConfParser.py +++ b/coalib/parsing/ConfParser.py @@ -8,6 +8,7 @@ from coalib.settings.Section import Section from coalib.settings.Setting import Setting from coalib.bearlib import deprecate_settings +from coalib.results.SourcePosition import SourcePosition class ConfParser: @@ -90,6 +91,7 @@ def __parse_lines(self, lines, origin): current_section = self.get_section(current_section_name) current_keys = [] no_section = True + line_number = 0 for line in lines: (section_name, @@ -98,6 +100,7 @@ def __parse_lines(self, lines, origin): append, comment) = self.line_parser._parse(line) + line_number += 1 if comment != '': self.__add_comment(current_section, comment, origin) @@ -135,7 +138,8 @@ def __parse_lines(self, lines, origin): current_section.add_or_create_setting( Setting(key, value, - origin, + SourcePosition( + str(origin), line=line_number), to_append=append, # Start ignoring PEP8Bear, PycodestyleBear* # they fail to resolve this @@ -149,7 +153,8 @@ def __parse_lines(self, lines, origin): True).add_or_create_setting( Setting(key, value, - origin, + SourcePosition( + str(origin), line=line_number), to_append=append, # Start ignoring PEP8Bear, PycodestyleBear* # they fail to resolve this diff --git a/coalib/results/SourcePosition.py b/coalib/results/SourcePosition.py index ae355f51ac..7a8e05f61a 100644 --- a/coalib/results/SourcePosition.py +++ b/coalib/results/SourcePosition.py @@ -25,6 +25,7 @@ def __init__(self, file: str, line=None, column=None): """ TextPosition.__init__(self, line, column) + self.filename = file self._file = abspath(file) @property @@ -36,3 +37,11 @@ def __json__(self, use_relpath=False): if use_relpath: _dict['file'] = relpath(_dict['file']) return _dict + + def __str__(self): + source_position = self.filename + if self.line is not None: + source_position += ':' + str(self.line) + if self.column is not None: + source_position += ':' + str(self.column) + return source_position diff --git a/coalib/settings/ConfigurationGathering.py b/coalib/settings/ConfigurationGathering.py index 5154c6b424..010e6aa3e8 100644 --- a/coalib/settings/ConfigurationGathering.py +++ b/coalib/settings/ConfigurationGathering.py @@ -367,8 +367,7 @@ def get_config_directory(section): However if its origin is already a directory this will be preserved: - >>> files = section['files'] - >>> files.origin = os.path.abspath('/tmp/dir/') + >>> files = Setting('files', '**', origin=os.path.abspath('/tmp/dir/')) >>> section.append(files) >>> os.makedirs(section['files'].origin, exist_ok=True) >>> get_config_directory(section) == section['files'].origin diff --git a/coalib/settings/Setting.py b/coalib/settings/Setting.py index 1af901867b..300bbb30a0 100644 --- a/coalib/settings/Setting.py +++ b/coalib/settings/Setting.py @@ -8,6 +8,7 @@ from coala_utils.string_processing.StringConverter import StringConverter from coalib.bearlib.languages.Language import Language, UnknownLanguageError from coalib.parsing.Globbing import glob_escape +from coalib.results.SourcePosition import SourcePosition def path(obj, *args, **kwargs): @@ -155,7 +156,7 @@ class Setting(StringConverter): def __init__(self, key, value, - origin: str = '', + origin: (str, SourcePosition) = '', strip_whitespaces: bool = True, list_delimiters: Iterable = (',', ';'), from_cli: bool = False, @@ -196,7 +197,7 @@ def __init__(self, self.from_cli = from_cli self.key = key - self.origin = str(origin) + self._origin = origin def __path__(self, origin=None, glob_escape_origin=False): """ @@ -296,3 +297,22 @@ def value(self): 'incomplete. Please access the value of the ' 'setting in a section to get the complete value.') return self._value + + @property + def origin(self): + """ + Returns the filename. + """ + if isinstance(self._origin, SourcePosition): + return self._origin.filename + else: + return self._origin + + @property + def line_number(self): + if isinstance(self._origin, SourcePosition): + return self._origin.line + else: + raise TypeError("Instantiated with str 'origin' " + 'which does not have line numbers. ' + 'Use SourcePosition for line numbers.') diff --git a/tests/parsing/ConfParserTest.py b/tests/parsing/ConfParserTest.py index 451f58e887..fd36cc2fa8 100644 --- a/tests/parsing/ConfParserTest.py +++ b/tests/parsing/ConfParserTest.py @@ -98,6 +98,11 @@ def test_parse_default_section_deprecated(self): self.assertRegex(self.cm.output[0], 'A setting does not have a section.') + # Check if line number is correctly set when + # no section is given + line_num = val.contents['setting'].line_number + self.assertEqual(line_num, 1) + def test_parse_foo_section(self): foo_should = OrderedDict([ ('a_default', 'val'), @@ -146,6 +151,13 @@ def test_parse_makefiles_section(self): self.assertEqual(val['comment1'].key, 'comment1') + # Check starting line number of + # settings in makefiles section. + line_num = val.contents['another'].line_number + self.assertEqual(line_num, 12) + line_num = val.contents['append'].line_number + self.assertEqual(line_num, 20) + def test_parse_empty_elem_strip_section(self): empty_elem_strip_should = OrderedDict([ ('a', 'a, b, c'), @@ -167,6 +179,22 @@ def test_parse_empty_elem_strip_section(self): is_dict[k] = str(val[k]) self.assertEqual(is_dict, empty_elem_strip_should) + # Check starting line number of + # settings in empty_elem_strip section. + line_num = val.contents['b'].line_number + self.assertEqual(line_num, 24) + + def test_line_number_name_section(self): + # Pop off the default, foo, makefiles and empty_elem_strip sections. + self.sections.popitem(last=False) + self.sections.popitem(last=False) + self.sections.popitem(last=False) + self.sections.popitem(last=False) + + key, val = self.sections.popitem(last=False) + line_num = val.contents['key1'].line_number + self.assertEqual(line_num, 30) + def test_remove_empty_iter_elements(self): # Test with empty-elem stripping. uut = ConfParser(remove_empty_iter_elements=True) diff --git a/tests/results/SourcePositionTest.py b/tests/results/SourcePositionTest.py index 29aacc4f7c..9fdf81de0f 100644 --- a/tests/results/SourcePositionTest.py +++ b/tests/results/SourcePositionTest.py @@ -25,12 +25,17 @@ def test_string_conversion(self): repr(uut), "') + self.assertEqual(str(uut), 'filename:1') uut = SourcePosition('None', None) self.assertRegex( repr(uut), "') + self.assertEqual(str(uut), 'None') + + uut = SourcePosition('filename', 3, 2) + self.assertEqual(str(uut), 'filename:3:2') def test_json(self): with prepare_file([''], None) as (_, filename): diff --git a/tests/settings/SettingTest.py b/tests/settings/SettingTest.py index a4251d6a40..7f3adf12f6 100644 --- a/tests/settings/SettingTest.py +++ b/tests/settings/SettingTest.py @@ -11,6 +11,7 @@ ) from coalib.parsing.DefaultArgParser import PathArg from coalib.parsing.Globbing import glob_escape +from coalib.results.SourcePosition import SourcePosition class SettingTest(unittest.TestCase): @@ -44,12 +45,24 @@ def test_path(self): origin='test' + os.path.sep), os.path.abspath(os.path.join('test', '22'))) + self.uut = Setting('key', '22\n', + SourcePosition('.' + os.path.sep), + strip_whitespaces=True) + self.assertEqual(path(self.uut), + os.path.abspath(os.path.join('.', '22'))) + def test_glob(self): self.uut = Setting('key', '.', origin=os.path.join('test (1)', 'somefile')) self.assertEqual(glob(self.uut), glob_escape(os.path.abspath('test (1)'))) + self.uut = Setting('key', '.', + origin=SourcePosition( + os.path.join('test (1)', 'somefile'))) + self.assertEqual(glob(self.uut), + glob_escape(os.path.abspath('test (1)'))) + def test_path_list(self): abspath = os.path.abspath('.') # Need to escape backslashes since we use list conversion @@ -58,6 +71,12 @@ def test_path_list(self): self.assertEqual(path_list(self.uut), [os.path.abspath(os.path.join('test', '.')), abspath]) + self.uut = Setting('key', '., ' + abspath.replace('\\', '\\\\'), + origin=SourcePosition( + os.path.join('test', 'somefile'))) + self.assertEqual(path_list(self.uut), + [os.path.abspath(os.path.join('test', '.')), abspath]) + def test_url(self): uut = Setting('key', 'http://google.com') self.assertEqual(url(uut), 'http://google.com') @@ -76,6 +95,13 @@ def test_glob_list(self): [glob_escape(os.path.abspath(os.path.join('test (1)', '.'))), abspath]) + self.uut = Setting('key', '.,' + abspath.replace('\\', '\\\\'), + origin=SourcePosition( + os.path.join('test (1)', 'somefile'))) + self.assertEqual(glob_list(self.uut), + [glob_escape(os.path.abspath( + os.path.join('test (1)', '.'))), abspath]) + def test_language(self): self.uut = Setting('key', 'python 3.4') result = language(self.uut) @@ -175,3 +201,14 @@ def test_value_getter(self): 'Iteration on this object is invalid'): self.uut = Setting('key', '1, 2, 3', '.', to_append=True) list(self.uut) + + def test_line_number(self): + self.uut = Setting('key', '22\n', origin=SourcePosition('filename', 3)) + self.assertEqual(self.uut.line_number, 3) + + with self.assertRaisesRegex(TypeError, + "Instantiated with str 'origin' " + 'which does not have line numbers. ' + 'Use SourcePosition for line numbers.'): + self.uut = Setting('key', '22\n', origin='filename') + self.uut.line_number