From 706cea8996eb73bca6b0ca287c1373835d11c0e4 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Mon, 13 Jan 2020 11:43:49 -0700 Subject: [PATCH] detect duplicate macro names in the same project, and raise an exception about it --- core/dbt/parser/results.py | 19 ++++++- core/dbt/ui/printer.py | 10 +++- .../local_dependency/macros/dep_macro.sql | 3 ++ .../macros/main_macro.sql | 4 ++ .../macros-bad-same/macros.sql | 5 ++ .../macros-bad-separate/one.sql | 2 + .../macros-bad-separate/two.sql | 2 + .../test_duplicate_macro.py | 52 +++++++++++++++++++ 8 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 test/integration/006_simple_dependency_test/local_dependency/macros/dep_macro.sql create mode 100644 test/integration/006_simple_dependency_test/macros/main_macro.sql create mode 100644 test/integration/025_duplicate_model_test/macros-bad-same/macros.sql create mode 100644 test/integration/025_duplicate_model_test/macros-bad-separate/one.sql create mode 100644 test/integration/025_duplicate_model_test/macros-bad-separate/two.sql create mode 100644 test/integration/025_duplicate_model_test/test_duplicate_macro.py diff --git a/core/dbt/parser/results.py b/core/dbt/parser/results.py index b594159730e..398150a4d37 100644 --- a/core/dbt/parser/results.py +++ b/core/dbt/parser/results.py @@ -12,8 +12,9 @@ from dbt.contracts.util import Writable, Replaceable from dbt.exceptions import ( raise_duplicate_resource_name, raise_duplicate_patch_name, - CompilationException, InternalException + CompilationException, InternalException, raise_compiler_error ) +from dbt.ui import printer from dbt.version import __version__ @@ -87,7 +88,21 @@ def add_disabled(self, source_file: SourceFile, node: ParsedNode): self.get_file(source_file).nodes.append(node.unique_id) def add_macro(self, source_file: SourceFile, macro: ParsedMacro): - # macros can be overwritten (should they be?) + if macro.unique_id in self.macros: + # detect that the macro exists and emit an error + other_path = self.macros[macro.unique_id].original_file_path + # subtract 2 for the "Compilation Error" indent + msg = printer.line_wrap_message( + f'''\ + dbt found two macros in the project "{macro.package_name}" with + the name "{macro.name}" (defined in + "{macro.original_file_path}" and "{other_path}"). Since these + resources have the same name, dbt will choose one to be called + when {macro.name} is used. To fix this, remove or change the + name of one of these macros.''', + subtract=2) + raise_compiler_error(msg) + self.macros[macro.unique_id] = macro self.get_file(source_file).macros.append(macro.unique_id) diff --git a/core/dbt/ui/printer.py b/core/dbt/ui/printer.py index 940e1a54ca1..f785d75c424 100644 --- a/core/dbt/ui/printer.py +++ b/core/dbt/ui/printer.py @@ -1,3 +1,5 @@ +import textwrap +import time from typing import Dict, Optional, Tuple from dbt.logger import GLOBAL_LOGGER as logger, DbtStatusMessage, TextOnly @@ -6,7 +8,6 @@ from dbt.utils import get_materialization import dbt.ui.colors -import time USE_COLORS = False @@ -380,3 +381,10 @@ def print_run_end_messages(results, early_exit: bool = False) -> None: print_run_result_error(warning, is_warning=True) print_run_status_line(results) + + +def line_wrap_message(msg: str, subtract: int = 0, dedent: bool = True) -> str: + width = PRINTER_WIDTH - subtract + if dedent: + msg = textwrap.dedent(msg) + return textwrap.fill(msg, width=width) diff --git a/test/integration/006_simple_dependency_test/local_dependency/macros/dep_macro.sql b/test/integration/006_simple_dependency_test/local_dependency/macros/dep_macro.sql new file mode 100644 index 00000000000..81e9a0faeef --- /dev/null +++ b/test/integration/006_simple_dependency_test/local_dependency/macros/dep_macro.sql @@ -0,0 +1,3 @@ +{% macro some_overridden_macro() -%} +100 +{%- endmacro %} diff --git a/test/integration/006_simple_dependency_test/macros/main_macro.sql b/test/integration/006_simple_dependency_test/macros/main_macro.sql new file mode 100644 index 00000000000..da8731a08da --- /dev/null +++ b/test/integration/006_simple_dependency_test/macros/main_macro.sql @@ -0,0 +1,4 @@ +{# This macro also exists in the dependency -dbt should be fine with that #} +{% macro some_overridden_macro() -%} +999 +{%- endmacro %} diff --git a/test/integration/025_duplicate_model_test/macros-bad-same/macros.sql b/test/integration/025_duplicate_model_test/macros-bad-same/macros.sql new file mode 100644 index 00000000000..6799a49c689 --- /dev/null +++ b/test/integration/025_duplicate_model_test/macros-bad-same/macros.sql @@ -0,0 +1,5 @@ +{% macro some_macro() %} +{% endmacro %} + +{% macro some_macro() %} +{% endmacro %} diff --git a/test/integration/025_duplicate_model_test/macros-bad-separate/one.sql b/test/integration/025_duplicate_model_test/macros-bad-separate/one.sql new file mode 100644 index 00000000000..85defd788ef --- /dev/null +++ b/test/integration/025_duplicate_model_test/macros-bad-separate/one.sql @@ -0,0 +1,2 @@ +{% macro some_macro() %} +{% endmacro %} diff --git a/test/integration/025_duplicate_model_test/macros-bad-separate/two.sql b/test/integration/025_duplicate_model_test/macros-bad-separate/two.sql new file mode 100644 index 00000000000..85defd788ef --- /dev/null +++ b/test/integration/025_duplicate_model_test/macros-bad-separate/two.sql @@ -0,0 +1,2 @@ +{% macro some_macro() %} +{% endmacro %} diff --git a/test/integration/025_duplicate_model_test/test_duplicate_macro.py b/test/integration/025_duplicate_model_test/test_duplicate_macro.py new file mode 100644 index 00000000000..aa927d2b83c --- /dev/null +++ b/test/integration/025_duplicate_model_test/test_duplicate_macro.py @@ -0,0 +1,52 @@ +import os +from dbt.exceptions import CompilationException +from test.integration.base import DBTIntegrationTest, use_profile + + +class TestDuplicateMacroEnabledSameFile(DBTIntegrationTest): + + @property + def schema(self): + return "duplicate_macro_025" + + @property + def models(self): + return "models-3" + + @property + def project_config(self): + return { + 'macro-paths': ['macros-bad-same'] + } + + @use_profile('postgres') + def test_postgres_duplicate_macros(self): + with self.assertRaises(CompilationException) as exc: + self.run_dbt(['compile']) + self.assertIn('some_macro', str(exc.exception)) + self.assertIn(os.path.join('macros-bad-same', 'macros.sql'), str(exc.exception)) + + +class TestDuplicateMacroEnabledDifferentFiles(DBTIntegrationTest): + + @property + def schema(self): + return "duplicate_macro_025" + + @property + def models(self): + return "models-3" + + @property + def project_config(self): + return { + 'macro-paths': ['macros-bad-separate'] + } + + @use_profile('postgres') + def test_postgres_duplicate_macros(self): + with self.assertRaises(CompilationException) as exc: + self.run_dbt(['compile']) + self.assertIn('some_macro', str(exc.exception)) + self.assertIn(os.path.join('macros-bad-separate', 'one.sql'), str(exc.exception)) + self.assertIn(os.path.join('macros-bad-separate', 'two.sql'), str(exc.exception))