Repository: sqlfluff/sqlfluff

Issue:
Return codes are inconsistent
### Search before asking

- [X] I searched the [issues](https://github.com/sqlfluff/sqlfluff/issues) and found no similar issues.


### What Happened

Working on #3431 - I noticed that we're inconsistent in our return codes.

In `commands.py` we call `sys.exit()` in 15 places (currently).

- Twice we call `sys.exit(0)` on success, at the end of `parse` and `lint` (`fix` is a handled differently, see below). ✔️ 
- Six times we call `sys.exit(1)` for a selection of things:
  - Not having `cProfiler` installed.
  - Failing to apply fixes
  - User Errors and OSError (in `PathAndUserErrorHandler`)
- Five times we call `sys.exit(66)` for a selection of things:
  - User Errors (including unknown dialect or failing to load a dialect or config)
  - If parsing failed when calling `parse`.
- Once we use `handle_files_with_tmp_or_prs_errors` to determine the exit code (which returns 1 or 0)
- Once we use `LintingResult.stats` to determine the exit code (which returns either 65 or 0)
- Once we do a mixture of the above (see end of `fix`)

This neither DRY, or consistent ... or helpful?

### Expected Behaviour

We should have consistent return codes for specific scenarios. There are up for discussion, but I would suggest:

- 0 for success (obviously)
- 1 for a fail which is error related: not having libraries installed, user errors etc...
- 65 for a linting fail (i.e. no errors in running, but issues were found in either parsing or linting).
- 66 for a fixing fail (i.e. we tried to fix errors but failed to do so for some reason).

These would be defined as constants at the top of `commands.py`.

### Observed Behaviour

see above

### How to reproduce

see above

### Dialect

N/A

### Version

Description is as per code in #3431

### Configuration

-

### Are you willing to work on and submit a PR to address the issue?

- [X] Yes I am willing to submit a PR!

### Code of Conduct

- [X] I agree to follow this project's [Code of Conduct](https://github.com/sqlfluff/sqlfluff/blob/main/CODE_OF_CONDUCT.md)


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/sqlfluff/sqlfluff/pull/3608?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff) Report
> Merging [#3608](https://codecov.io/gh/sqlfluff/sqlfluff/pull/3608?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff) (9a88e82) into [main](https://codecov.io/gh/sqlfluff/sqlfluff/commit/d376f23d72df7aeceb17acbc7010fe44398c0e49?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff) (d376f23) will **not change** coverage.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff            @@
##              main     #3608   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          174       175    +1     
  Lines        13244     13250    +6     
=========================================
+ Hits         13244     13250    +6     
```


| [Impacted Files](https://codecov.io/gh/sqlfluff/sqlfluff/pull/3608?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff) | Coverage Δ | |
|---|---|---|
| [src/sqlfluff/cli/\_\_init\_\_.py](https://codecov.io/gh/sqlfluff/sqlfluff/pull/3608/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff#diff-c3JjL3NxbGZsdWZmL2NsaS9fX2luaXRfXy5weQ==) | `100.00% <100.00%> (ø)` | |
| [src/sqlfluff/cli/commands.py](https://codecov.io/gh/sqlfluff/sqlfluff/pull/3608/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff#diff-c3JjL3NxbGZsdWZmL2NsaS9jb21tYW5kcy5weQ==) | `100.00% <100.00%> (ø)` | |
| [src/sqlfluff/cli/formatters.py](https://codecov.io/gh/sqlfluff/sqlfluff/pull/3608/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff#diff-c3JjL3NxbGZsdWZmL2NsaS9mb3JtYXR0ZXJzLnB5) | `100.00% <100.00%> (ø)` | |
| [src/sqlfluff/core/linter/linting\_result.py](https://codecov.io/gh/sqlfluff/sqlfluff/pull/3608/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff#diff-c3JjL3NxbGZsdWZmL2NvcmUvbGludGVyL2xpbnRpbmdfcmVzdWx0LnB5) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/sqlfluff/sqlfluff/pull/3608?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/sqlfluff/sqlfluff/pull/3608?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff). Last update [d376f23...9a88e82](https://codecov.io/gh/sqlfluff/sqlfluff/pull/3608?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff).


Patch:
diff --git a/src/sqlfluff/cli/__init__.py b/src/sqlfluff/cli/__init__.py
index 651d690e6..58039a256 100644
--- a/src/sqlfluff/cli/__init__.py
+++ b/src/sqlfluff/cli/__init__.py
@@ -1,1 +1,6 @@
 """init py for cli."""
+
+
+EXIT_SUCCESS = 0
+EXIT_FAIL = 1
+EXIT_ERROR = 2
diff --git a/src/sqlfluff/cli/commands.py b/src/sqlfluff/cli/commands.py
index d09918661..491bf8a5b 100644
--- a/src/sqlfluff/cli/commands.py
+++ b/src/sqlfluff/cli/commands.py
@@ -22,6 +22,7 @@ import colorama
 from tqdm import tqdm
 from sqlfluff.cli.autocomplete import shell_completion_enabled, dialect_shell_complete
 
+from sqlfluff.cli import EXIT_SUCCESS, EXIT_ERROR, EXIT_FAIL
 from sqlfluff.cli.formatters import (
     format_linting_result_header,
     OutputStreamFormatter,
@@ -154,7 +155,7 @@ class PathAndUserErrorHandler:
                     Color.red,
                 )
             )
-            sys.exit(1)
+            sys.exit(EXIT_ERROR)
         elif exc_type is SQLFluffUserError:
             click.echo(
                 "\nUser Error: "
@@ -163,7 +164,7 @@ class PathAndUserErrorHandler:
                     Color.red,
                 )
             )
-            sys.exit(1)
+            sys.exit(EXIT_ERROR)
 
 
 def common_options(f: Callable) -> Callable:
@@ -335,7 +336,7 @@ def get_config(
                     color=Color.red,
                 )
             )
-            sys.exit(66)
+            sys.exit(EXIT_ERROR)
         except KeyError:
             click.echo(
                 OutputStreamFormatter.colorize_helper(
@@ -344,7 +345,7 @@ def get_config(
                     color=Color.red,
                 )
             )
-            sys.exit(66)
+            sys.exit(EXIT_ERROR)
     from_root_kwargs = {}
     if "require_dialect" in kwargs:
         from_root_kwargs["require_dialect"] = kwargs.pop("require_dialect")
@@ -365,7 +366,7 @@ def get_config(
                 color=Color.red,
             )
         )
-        sys.exit(66)
+        sys.exit(EXIT_ERROR)
 
 
 def get_linter_and_formatter(
@@ -380,7 +381,7 @@ def get_linter_and_formatter(
             dialect_selector(dialect)
     except KeyError:  # pragma: no cover
         click.echo(f"Error: Unknown dialect '{cfg.get('dialect')}'")
-        sys.exit(66)
+        sys.exit(EXIT_ERROR)
     formatter = OutputStreamFormatter(
         output_stream=output_stream or make_output_stream(cfg),
         nocolor=cfg.get("nocolor"),
@@ -635,7 +636,7 @@ def lint(
             formatter.completion_message()
         sys.exit(result.stats()["exit code"])
     else:
-        sys.exit(0)
+        sys.exit(EXIT_SUCCESS)
 
 
 def do_fixes(lnt, result, formatter=None, **kwargs):
@@ -730,7 +731,7 @@ def fix(
     verbose = config.get("verbose")
     progress_bar_configuration.disable_progress_bar = disable_progress_bar
 
-    exit_code = 0
+    exit_code = EXIT_SUCCESS
 
     formatter.dispatch_config(lnt)
 
@@ -780,7 +781,7 @@ def fix(
             )
 
         click.echo(stdout, nl=False)
-        sys.exit(1 if templater_error or unfixable_error else exit_code)
+        sys.exit(EXIT_FAIL if templater_error or unfixable_error else exit_code)
 
     # Lint the paths (not with the fix argument at this stage), outputting as we go.
     click.echo("==== finding fixable violations ====")
@@ -816,7 +817,7 @@ def fix(
                 fixed_file_suffix=fixed_suffix,
             )
             if not success:
-                sys.exit(1)  # pragma: no cover
+                sys.exit(EXIT_FAIL)  # pragma: no cover
         else:
             click.echo(
                 "Are you sure you wish to attempt to fix these? [Y/n] ", nl=False
@@ -833,16 +834,16 @@ def fix(
                     fixed_file_suffix=fixed_suffix,
                 )
                 if not success:
-                    sys.exit(1)  # pragma: no cover
+                    sys.exit(EXIT_FAIL)  # pragma: no cover
                 else:
                     formatter.completion_message()
             elif c == "n":
                 click.echo("Aborting...")
-                exit_code = 1
+                exit_code = EXIT_FAIL
             else:  # pragma: no cover
                 click.echo("Invalid input, please enter 'Y' or 'N'")
                 click.echo("Aborting...")
-                exit_code = 1
+                exit_code = EXIT_FAIL
     else:
         click.echo("==== no fixable linting violations found ====")
         formatter.completion_message()
@@ -851,7 +852,7 @@ def fix(
         (
             dict(types=SQLLintError, fixable=False),
             "  [{} unfixable linting violations found]",
-            1,
+            EXIT_FAIL,
         ),
     ]
     for num_violations_kwargs, message_format, error_level in error_types:
@@ -986,7 +987,7 @@ def parse(
             import cProfile
         except ImportError:  # pragma: no cover
             click.echo("The cProfiler is not available on your platform.")
-            sys.exit(1)
+            sys.exit(EXIT_ERROR)
         pr = cProfile.Profile()
         pr.enable()
 
@@ -1053,9 +1054,9 @@ def parse(
         click.echo("\n".join(profiler_buffer.getvalue().split("\n")[:50]))
 
     if violations_count > 0 and not nofail:
-        sys.exit(66)  # pragma: no cover
+        sys.exit(EXIT_FAIL)  # pragma: no cover
     else:
-        sys.exit(0)
+        sys.exit(EXIT_SUCCESS)
 
 
 # This "__main__" handler allows invoking SQLFluff using "python -m", which
diff --git a/src/sqlfluff/cli/formatters.py b/src/sqlfluff/cli/formatters.py
index d17ae3962..45eae7e04 100644
--- a/src/sqlfluff/cli/formatters.py
+++ b/src/sqlfluff/cli/formatters.py
@@ -6,6 +6,7 @@ from typing import List, Optional, Tuple, Union
 import click
 from colorama import Style
 
+from sqlfluff.cli import EXIT_FAIL, EXIT_SUCCESS
 from sqlfluff.cli.helpers import (
     get_package_version,
     get_python_version,
@@ -14,6 +15,7 @@ from sqlfluff.cli.helpers import (
     wrap_field,
 )
 from sqlfluff.cli.outputstream import OutputStream
+
 from sqlfluff.core import SQLBaseError, FluffConfig, Linter, TimingSummary
 from sqlfluff.core.enums import Color
 from sqlfluff.core.linter import LintedFile, LintingResult, ParsedString
@@ -517,7 +519,7 @@ class OutputStreamFormatter:
                         color,
                     )
                 )
-        return 1 if num_filtered_errors else 0
+        return EXIT_FAIL if num_filtered_errors else EXIT_SUCCESS
 
     def print_out_violations_and_timing(
         self,
diff --git a/src/sqlfluff/core/linter/linting_result.py b/src/sqlfluff/core/linter/linting_result.py
index f600d0569..45a7bc580 100644
--- a/src/sqlfluff/core/linter/linting_result.py
+++ b/src/sqlfluff/core/linter/linting_result.py
@@ -11,6 +11,7 @@ from typing import (
 )
 from typing_extensions import Literal
 
+from sqlfluff.cli import EXIT_FAIL, EXIT_SUCCESS
 
 from sqlfluff.core.errors import (
     CheckTuple,
@@ -23,8 +24,6 @@ from sqlfluff.core.timing import TimingSummary
 
 # Classes needed only for type checking
 from sqlfluff.core.parser.segments.base import BaseSegment
-
-
 from sqlfluff.core.linter.linted_dir import LintedDir
 
 
@@ -133,7 +132,9 @@ class LintingResult:
             all_stats["unclean rate"] = 0
         all_stats["clean files"] = all_stats["clean"]
         all_stats["unclean files"] = all_stats["unclean"]
-        all_stats["exit code"] = 65 if all_stats["violations"] > 0 else 0
+        all_stats["exit code"] = (
+            EXIT_FAIL if all_stats["violations"] > 0 else EXIT_SUCCESS
+        )
         all_stats["status"] = "FAIL" if all_stats["violations"] > 0 else "PASS"
         return all_stats
 


Test patch:
diff --git a/test/cli/commands_test.py b/test/cli/commands_test.py
index f9c09309d..b9b29b8f5 100644
--- a/test/cli/commands_test.py
+++ b/test/cli/commands_test.py
@@ -71,7 +71,7 @@ L:   5 | P:  13 | L031 | Avoid aliases in from clauses and join conditions.
 def test__cli__command_directed():
     """Basic checking of lint functionality."""
     result = invoke_assert_code(
-        ret_code=65,
+        ret_code=1,
         args=[
             lint,
             [
@@ -95,7 +95,7 @@ def test__cli__command_dialect():
     """Check the script raises the right exception on an unknown dialect."""
     # The dialect is unknown should be a non-zero exit code
     invoke_assert_code(
-        ret_code=66,
+        ret_code=2,
         args=[
             lint,
             [
@@ -112,7 +112,7 @@ def test__cli__command_no_dialect():
     """Check the script raises the right exception no dialect."""
     # The dialect is unknown should be a non-zero exit code
     result = invoke_assert_code(
-        ret_code=1,
+        ret_code=2,
         args=[
             lint,
             ["-"],
@@ -129,7 +129,7 @@ def test__cli__command_parse_error_dialect_explicit_warning():
     # and a human-readable warning should be dislayed.
     # Dialect specified as commandline option.
     result = invoke_assert_code(
-        ret_code=66,
+        ret_code=1,
         args=[
             parse,
             [
@@ -152,7 +152,7 @@ def test__cli__command_parse_error_dialect_implicit_warning():
     # and a human-readable warning should be dislayed.
     # Dialect specified in .sqlfluff config.
     result = invoke_assert_code(
-        ret_code=66,
+        ret_code=1,
         args=[
             # Config sets dialect to tsql
             parse,
@@ -173,7 +173,7 @@ def test__cli__command_parse_error_dialect_implicit_warning():
 def test__cli__command_dialect_legacy():
     """Check the script raises the right exception on a legacy dialect."""
     result = invoke_assert_code(
-        ret_code=66,
+        ret_code=2,
         args=[
             lint,
             [
@@ -190,7 +190,7 @@ def test__cli__command_dialect_legacy():
 def test__cli__command_extra_config_fail():
     """Check the script raises the right exception non-existent extra config path."""
     result = invoke_assert_code(
-        ret_code=66,
+        ret_code=2,
         args=[
             lint,
             [
@@ -429,7 +429,7 @@ def test__cli__command_lint_parse(command):
                 ["test/fixtures/cli/unknown_jinja_tag/test.sql", "-vvvvvvv"],
                 "y",
             ),
-            65,
+            1,
         ),
     ],
 )
@@ -461,7 +461,7 @@ def test__cli__command_lint_skip_ignore_files():
             "--disregard-sqlfluffignores",
         ],
     )
-    assert result.exit_code == 65
+    assert result.exit_code == 1
     assert "L009" in result.output.strip()
 
 
@@ -488,7 +488,7 @@ def test__cli__command_lint_ignore_local_config():
             "test/fixtures/cli/ignore_local_config/ignore_local_config_test.sql",
         ],
     )
-    assert result.exit_code == 65
+    assert result.exit_code == 1
     assert "L012" in result.output.strip()
 
 
@@ -561,7 +561,7 @@ def generic_roundtrip_test(
     old_mode = stat.S_IMODE(status.st_mode)
     # Check that we first detect the issue
     invoke_assert_code(
-        ret_code=65, args=[lint, ["--dialect=ansi", "--rules", rulestring, filepath]]
+        ret_code=1, args=[lint, ["--dialect=ansi", "--rules", rulestring, filepath]]
     )
     # Fix the file (in force mode)
     if force:
@@ -997,7 +997,7 @@ def test__cli__command_fix_stdin_error_exit_code(
     "rule,fname,prompt,exit_code,fix_exit_code",
     [
         ("L001", "test/fixtures/linter/indentation_errors.sql", "y", 0, 0),
-        ("L001", "test/fixtures/linter/indentation_errors.sql", "n", 65, 1),
+        ("L001", "test/fixtures/linter/indentation_errors.sql", "n", 1, 1),
     ],
 )
 def test__cli__command__fix_no_force(rule, fname, prompt, exit_code, fix_exit_code):
@@ -1075,7 +1075,7 @@ def test__cli__command_parse_serialize_from_stdin(serialize, write_file, tmp_pat
                     ],
                 }
             ],
-            65,
+            1,
         ),
     ],
 )
@@ -1115,7 +1115,7 @@ def test__cli__command_lint_serialize_from_stdin(serialize, sql, expected, exit_
 )
 def test__cli__command_fail_nice_not_found(command):
     """Check commands fail as expected when then don't find files."""
-    result = invoke_assert_code(args=command, ret_code=1)
+    result = invoke_assert_code(args=command, ret_code=2)
     assert "could not be accessed" in result.output
 
 
@@ -1180,7 +1180,7 @@ def test__cli__command_lint_serialize_multiple_files(serialize, write_file, tmp_
     # note the file is in here twice. two files = two payloads.
     result = invoke_assert_code(
         args=[lint, cmd_args],
-        ret_code=65,
+        ret_code=1,
     )
 
     if write_file:
@@ -1226,7 +1226,7 @@ def test__cli__command_lint_serialize_github_annotation():
                 "--disable_progress_bar",
             ),
         ],
-        ret_code=65,
+        ret_code=1,
     )
     result = json.loads(result.output)
     assert result == [
@@ -1337,7 +1337,7 @@ def test__cli__command_lint_serialize_github_annotation_native():
                 "--disable_progress_bar",
             ),
         ],
-        ret_code=65,
+        ret_code=1,
     )
 
     assert result.output == "\n".join(
@@ -1381,7 +1381,7 @@ def test__cli__command_lint_serialize_annotation_level_error_failure_equivalent(
                 "--disable_progress_bar",
             ),
         ],
-        ret_code=65,
+        ret_code=1,
     )
 
     result_failure = invoke_assert_code(
@@ -1396,7 +1396,7 @@ def test__cli__command_lint_serialize_annotation_level_error_failure_equivalent(
                 "--disable_progress_bar",
             ),
         ],
-        ret_code=65,
+        ret_code=1,
     )
 
     assert result_error.output == result_failure.output
@@ -1450,7 +1450,7 @@ def test_cli_encoding(encoding, method, expect_success, tmpdir):
         shutil.copy(sql_path, tmpdir)
         options = [str(tmpdir / "encoding_test.sql")]
     result = invoke_assert_code(
-        ret_code=65,
+        ret_code=1,
         args=[
             lint,
             options,
@@ -1479,7 +1479,7 @@ def test_cli_no_disable_noqa_flag():
 def test_cli_disable_noqa_flag():
     """Test that --disable_noqa flag ignores inline noqa comments."""
     result = invoke_assert_code(
-        ret_code=65,
+        ret_code=1,
         args=[
             lint,
             [
@@ -1563,7 +1563,7 @@ class TestProgressBars:
     ) -> None:
         """When progress bar is enabled, there should be some tracks in output."""
         result = invoke_assert_code(
-            ret_code=65,
+            ret_code=1,
             args=[
                 lint,
                 [
diff --git a/test/rules/std_roundtrip_test.py b/test/rules/std_roundtrip_test.py
index f9a0da7e4..ef356285f 100644
--- a/test/rules/std_roundtrip_test.py
+++ b/test/rules/std_roundtrip_test.py
@@ -34,7 +34,7 @@ def generic_roundtrip_test(source_file, rulestring):
     runner = CliRunner()
     # Check that we first detect the issue
     result = runner.invoke(lint, ["--rules", rulestring, "--dialect=ansi", filepath])
-    assert result.exit_code == 65
+    assert result.exit_code == 1
     # Fix the file (in force mode)
     result = runner.invoke(
         fix, ["--rules", rulestring, "--dialect=ansi", "-f", filepath]
@@ -80,7 +80,7 @@ def jinja_roundtrip_test(
     result = runner.invoke(
         lint, ["--rules", rulestring, "--dialect=ansi", sql_filepath]
     )
-    assert result.exit_code == 65
+    assert result.exit_code == 1
     # Fix the file (in force mode)
     result = runner.invoke(
         fix, ["--rules", rulestring, "-f", "--dialect=ansi", sql_filepath]