Merge pull request #2045 from fishtown-analytics/feature/detect-dupli…

…cate-macros

Detect duplicate macros (#1891)

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,33 @@ 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
+            # note that the line wrap eats newlines, so if you want newlines,
+            # this is the result :(
+            msg = '\n'.join([
+                printer.line_wrap_message(
+                    f'''\
+                    dbt found two macros named "{macro.name}" in the project
+                    "{macro.package_name}".
+                    ''',
+                    subtract=2
+                ),
+                '',
+                printer.line_wrap_message(
+                    f'''\
+                    To fix this error, rename or remove one of the following
+                    macros:
+                    ''',
+                    subtract=2
+                ),
+                f'   - {macro.original_file_path}',
+                f'   - {other_path}'
+            ])
+            raise_compiler_error(msg)
+
         self.macros[macro.unique_id] = macro
         self.get_file(source_file).macros.append(macro.unique_id)
 

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)

test/integration/006_simple_dependency_test/local_dependency/macros/dep_macro.sql

@@ -0,0 +1,3 @@
+{% macro some_overridden_macro() -%}
+100
+{%- endmacro %}

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 %}

test/integration/025_duplicate_model_test/macros-bad-same/macros.sql

@@ -0,0 +1,5 @@
+{% macro some_macro() %}
+{% endmacro %}
+
+{% macro some_macro() %}
+{% endmacro %}

test/integration/025_duplicate_model_test/macros-bad-separate/one.sql

@@ -0,0 +1,2 @@
+{% macro some_macro() %}
+{% endmacro %}

test/integration/025_duplicate_model_test/macros-bad-separate/two.sql

@@ -0,0 +1,2 @@
+{% macro some_macro() %}
+{% endmacro %}

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))