Repository: dbt-labs/dbt-core
Pull request: Raise proper error message if duplicate macros found
resolves #2449 

<!---
  Include the number of the issue addressed by this PR above if applicable.
  PRs for code changes without an associated issue *will not be merged*.
  See CONTRIBUTING.md for more information.

  Example:
    resolves #1234
-->


### Description

<!--- Describe the Pull Request here -->


### Checklist
 - [x] I have signed the [CLA](https://docs.getdbt.com/docs/contributor-license-agreements)
 - [x] I have run this code in development and it appears to resolve the stated issue
 - [x] This PR includes tests, or tests are not required/relevant for this PR
 - [x] I have updated the `CHANGELOG.md` and added information about my change to the "dbt next" section.


Problem statement:
Raise a better error message if dbt parses the same file twice due to misconfiguration
### Describe the feature
If a macro is addressed twice in a dbt project, then dbt will fail to compile with:
```
Compilation Error
  dbt found two macros named "<macro name>" in the project "projectname".
   To fix this error, rename or remove one of the following macros:
      - dbt_modules/package/macros/my_macro.sql
      - dbt_modules/package/macros/my_macro.sql
```

This is confusing because there is actually on a single file and the macro is _not_ defined twice, dbt just doesn't recognize that it has already parsed the file.

The root cause of an error like this is a misconfiguration like:
```yml
macro-paths: [macros, dbt_modules/package/macros]
```

Instead, dbt should raise the following error if a file is parsed twice in parsing:

```
Compilation Error
  The file <filepath> was parsed multiple times by dbt.
  This error happens when a path is duplicated in a dbt_project.yml configuration.
  Check your `dbt_project.yml` file path configurations and remove any
  duplicated paths to fix this error.
```

### Describe alternatives you've considered
We could optionally _allow_ the duplicate paths, raise a warning, and continue on. I'm amenable to that, but I don't think there's any real merit to supporting it!

### Who will this benefit?
Clearer error messages are good for everyone

Hints from issue discussion:

Commit message: Fix failing test

Patch to review:
diff --git a/core/dbt/parser/results.py b/core/dbt/parser/results.py
index f2fc3b9d5f1..2bc01cfa50d 100644
--- a/core/dbt/parser/results.py
+++ b/core/dbt/parser/results.py
@@ -125,7 +125,7 @@ def add_macro(self, source_file: SourceFile, macro: ParsedMacro):
             # subtract 2 for the "Compilation Error" indent
             # note that the line wrap eats newlines, so if you want newlines,
             # this is the result :(
-            msg = line_wrap_message(
+            dup_macro_msg = line_wrap_message(
                 f'''\
                 dbt found two macros named "{macro.name}" in the project
                 "{macro.package_name}".
@@ -140,6 +140,21 @@ def add_macro(self, source_file: SourceFile, macro: ParsedMacro):
                 ''',
                 subtract=2
             )
+            dup_path_msg = line_wrap_message(
+                f"""\
+                The file {macro.original_file_path} for macro {macro.name}
+                was parsed multiple times by dbt. This error happens when a
+                path is duplicated in a dbt_project.yml configuration.
+                Check your `dbt_project.yml` file path configurations and
+                remove any duplicated paths to fix this error
+                """,
+                subtract=2,
+            )
+            msg = (
+                dup_path_msg
+                if macro.original_file_path == other_path
+                else dup_macro_msg
+            )
             raise_compiler_error(msg)
 
         self.macros[macro.unique_id] = macro


Code review comments:
diff_hunk: @@ -140,6 +140,21 @@ def add_macro(self, source_file: SourceFile, macro: ParsedMacro):
                 ''',
                 subtract=2
             )
+            dup_path_msg = line_wrap_message(
+                f"""\
+                The file {macro.original_file_path} for macro {macro.name}
+                was parsed multiple times by dbt. This error happens when a
+                path is duplicated in a dbt_project.yml configuration.
+                Check your `dbt_project.yml` file path configurations and
+                remove any duplicated paths to fix this error
path: core/dbt/parser/results.py
text: @user1:
The conditional check (`macro.original_file_path == other_path`) will also return `True` if a macro is defined multiple time, with the same name, in one file. Rather than try to disambiguate between these errors, I think it's fine to mention both possibilities in the error message:

```suggestion
                The macro {macro.name} in file {macro.original_file_path}
                was parsed multiple times by dbt. This error happens when a
                macro is defined multiple times in the same file, or when a
                path is duplicated in a dbt_project.yml configuration.
                To fix this error, check:
                
                    - {macro.original_file_path}
                
                    - The `macro-paths` configurations in dbt_project.yml
```

@author:
@user1 thanks for the quick feedback. I was considering the case where the macro was in two file paths, for eg in `macros` directory and also in say `macros/some/other/dir`. In that case `macro.original_file_path == other_path` should not return `True`. Do we not want to address that case?

@user1:
Yes, I completely agree with your rationale.

1. In the case where `original_file_path != other_path`, we should definitely return the `dup_macro_msg` error message that tells the user the path of each file in which the same-named macro is defined. (I believe we already do this.)

2. In the case where `original_file_path == other_path`, that means either:
a. The same file is parsed more than once, on account of a mistake in `macro-paths`
b. The same-named macro is defined more than once in the same file (nothing to do with `macro-paths`)

The preexisting error message handles only case 1. The current PR adds better error handling for case 2a; I believe the error message should mention the possibility of 2b as well.

@author:
Ah, I get what you are saying now. Thanks, makes sense! I have updated with changes and also added to changelog