Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid displaying syntax error as log message #11902

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Jun 17, 2024

Summary

Follow-up to #11901

This PR avoids displaying the syntax errors as log message now that the E999 diagnostic cannot be disabled.

For context on why this was added, refer to #2505. Basically, we would allow ignoring the syntax error diagnostic because certain syntax feature weren't supported back then like match statement. And, if a user ignored E999, Ruff would give no feedback if the source code contained any syntax error. So, this log message was a way to indicate to the user even if E999 was disabled.

The current state of the parser is such that (a) it matches with the latest grammar and (b) it's easy to add support for any new syntax.

Note: This PR doesn't remove the DisplayParseError struct because it's still being used by the formatter.

Test Plan

Update existing snapshots from the integration tests.

@dhruvmanila dhruvmanila force-pushed the dhruv/syntax-error-2 branch 2 times, most recently from 58fb82b to 72b9853 Compare June 18, 2024 12:12
@dhruvmanila dhruvmanila force-pushed the dhruv/syntax-error-3 branch 3 times, most recently from 085e9a4 to 9bc3076 Compare June 18, 2024 12:33
Copy link
Contributor

github-actions bot commented Jun 18, 2024

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+473 -36331 violations, +0 -0 fixes in 23 projects; 1 project error; 26 projects unchanged)

DisnakeDev/disnake (+4 -0 violations, +0 -0 fixes)

+ disnake/ext/commands/params.py:807:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ tests/ext/commands/test_params.py:143:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ tests/ext/commands/test_params.py:146:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ tests/ext/commands/test_params.py:214:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks

RasaHQ/rasa (+12 -0 violations, +0 -0 fixes)

+ .github/tests/test_validate_gpus.py:26:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ rasa/shared/utils/io.py:66:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ rasa/shared/utils/io.py:68:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ rasa/shared/utils/io.py:93:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ tests/cli/test_utils.py:458:13: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ tests/cli/test_utils.py:513:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ tests/conftest.py:891:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ tests/core/test_actions.py:2811:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ tests/shared/core/test_domain.py:1856:13: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ tests/shared/core/training_data/story_reader/test_yaml_story_reader.py:272:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
... 2 additional changes omitted for project

alteryx/featuretools (+3 -0 violations, +0 -0 fixes)

+ featuretools/tests/entityset_tests/test_es.py:714:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ featuretools/tests/entityset_tests/test_serialization.py:130:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ featuretools/tests/entityset_tests/test_serialization.py:131:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks

PlasmaPy/PlasmaPy (+2 -0 violations, +0 -0 fixes)

+ src/plasmapy/diagnostics/langmuir.py:1396:23: NPY201 `np.trapz` will be removed in NumPy 2.0. Use `numpy.trapezoid` on NumPy 2.0, or ignore this warning on earlier versions.
+ tests/particles/test_exceptions.py:1040:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks

apache/airflow (+182 -25349 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

- airflow/api/auth/backend/kerberos_auth.py:69:18: ANN101 Missing type annotation for `self` in method
- airflow/api/client/api_client.py:28:18: ANN101 Missing type annotation for `self` in method
- airflow/api/client/api_client.py:34:21: ANN101 Missing type annotation for `self` in method
- airflow/api/client/api_client.py:46:20: ANN101 Missing type annotation for `self` in method
- airflow/api/client/api_client.py:53:18: ANN101 Missing type annotation for `self` in method
- airflow/api/client/api_client.py:60:19: ANN101 Missing type annotation for `self` in method
... 24625 additional changes omitted for rule ANN101
- airflow/callbacks/callback_requests.py:57:19: ANN102 Missing type annotation for `cls` in classmethod
- airflow/callbacks/callback_requests.py:95:19: ANN102 Missing type annotation for `cls` in classmethod
+ airflow/cli/commands/dag_command.py:309:14: S603 `subprocess` call: check for execution of untrusted input
- airflow/cli/commands/dag_command.py:309:31: S603 `subprocess` call: check for execution of untrusted input
+ airflow/cli/commands/info_command.py:199:18: S603 `subprocess` call: check for execution of untrusted input
- airflow/cli/commands/info_command.py:199:35: S603 `subprocess` call: check for execution of untrusted input
+ airflow/cli/commands/internal_api_command.py:166:17: S603 `subprocess` call: check for execution of untrusted input
- airflow/cli/commands/internal_api_command.py:166:34: S603 `subprocess` call: check for execution of untrusted input
... 297 additional changes omitted for rule S603
- airflow/cli/commands/standalone_command.py:51:20: ANN102 Missing type annotation for `cls` in classmethod
- airflow/dag_processing/manager.py:498:9: ANN102 Missing type annotation for `cls` in classmethod
- airflow/dag_processing/processor.py:422:21: ANN102 Missing type annotation for `cls` in classmethod
- airflow/dag_processing/processor.py:803:21: ANN102 Missing type annotation for `cls` in classmethod
... 539 additional changes omitted for rule ANN102
+ airflow/example_dags/example_kubernetes_executor.py:132:35: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
- airflow/example_dags/example_kubernetes_executor.py:132:45: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+ airflow/example_dags/example_kubernetes_executor.py:94:27: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
- airflow/example_dags/example_kubernetes_executor.py:94:37: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+ airflow/models/abstractoperator.py:439:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ airflow/models/abstractoperator.py:441:14: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ airflow/models/abstractoperator.py:443:14: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ airflow/providers/apache/beam/hooks/beam.py:575:25: S604 Function call with `shell=True` parameter identified, security issue
- airflow/providers/apache/beam/hooks/beam.py:577:13: S604 Function call with `shell=True` parameter identified, security issue
- airflow/providers/microsoft/azure/hooks/msgraph.py:327:12: PLR1701 [*] Merge `isinstance` calls: `isinstance(data, (BytesIO, bytes, str))`
- airflow/serialization/pydantic/dag.py:45:9: PLR1701 [*] Merge `isinstance` calls
- airflow/serialization/pydantic/taskinstance.py:70:8: PLR1701 [*] Merge `isinstance` calls: `isinstance(x, (BaseOperator, MappedOperator))`
+ airflow/utils/pydantic.py:60:13: D107 Missing docstring in `__init__`
- airflow/utils/pydantic.py:60:13: D107 Missing docstring in `__init__`
+ dev/breeze/src/airflow_breeze/commands/kubernetes_commands.py:1082:13: S604 Function call with `shell=True` parameter identified, security issue
- dev/breeze/src/airflow_breeze/commands/kubernetes_commands.py:1091:17: S604 Function call with `shell=True` parameter identified, security issue
... 25497 additional changes omitted for project

bokeh/bokeh (+40 -4126 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

- examples/output/apis/autoload_static.py:32:20: ANN101 Missing type annotation for `self` in method
- examples/output/apis/autoload_static.py:34:13: ANN101 Missing type annotation for `self` in method
- examples/output/apis/autoload_static.py:41:20: ANN101 Missing type annotation for `self` in method
- examples/output/apis/autoload_static.py:43:13: ANN101 Missing type annotation for `self` in method
+ examples/output/apis/server_document/flask_server.py:45:17: S603 `subprocess` call: check for execution of untrusted input
- examples/output/apis/server_document/flask_server.py:46:5: S603 `subprocess` call: check for execution of untrusted input
- examples/server/api/tornado_embed.py:15:13: ANN101 Missing type annotation for `self` in method
- examples/server/app/server_auth/auth.py:27:13: ANN101 Missing type annotation for `self` in method
... 4003 additional changes omitted for rule ANN101
+ release/system.py:43:18: S602 `subprocess` call with `shell=True` identified, security issue
- release/system.py:43:34: S602 `subprocess` call with `shell=True` identified, security issue
... 4156 additional changes omitted for project

freedomofpress/securedrop (+2 -3 violations, +0 -0 fixes)

- admin/securedrop_admin/__init__.py:598:12: E721 Do not compare types, use `isinstance()`
- admin/securedrop_admin/__init__.py:600:12: E721 Do not compare types, use `isinstance()`
- admin/securedrop_admin/__init__.py:604:12: E721 Do not compare types, use `isinstance()`
+ devops/scripts/verify-mo.py:116:16: S602 `subprocess` call with `shell=True` identified, security issue
+ devops/scripts/verify-mo.py:120:26: RUF100 [*] Unused `noqa` directive (unused: `S602`)

fronzbot/blinkpy (+2 -0 violations, +0 -0 fixes)

+ blinksync/blinksync.py:58:53: F811 Redefinition of unused `working` from line 55
+ blinksync/blinksync.py:81:53: F811 Redefinition of unused `working` from line 78

ibis-project/ibis (+11 -0 violations, +0 -0 fixes)

+ ibis/backends/dask/convert.py:28:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ ibis/backends/pandas/convert.py:28:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ ibis/backends/tests/test_client.py:1219:13: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ ibis/expr/types/strings.py:1678:9: F811 Redefinition of unused `__mul__` from line 638
+ ibis/expr/types/strings.py:417:9: F811 Redefinition of unused `initcap` from line 412
+ ibis/tests/benchmarks/test_benchmarks.py:720:34: F811 Redefinition of unused `con` from line 705
+ ibis/tests/expr/test_sql_builtins.py:116:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ ibis/tests/expr/test_sql_builtins.py:117:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
+ ibis/tests/expr/test_sql_builtins.py:137:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
... 3 additional changes omitted for rule E721
... 2 additional changes omitted for project

... Truncated remaining completed project reports due to GitHub comment length restrictions

demisto/content (error)

ruff failed
  Cause: Failed to parse /home/runner/work/ruff/ruff/checkouts/demisto:content/pyproject.toml
  Cause: TOML parse error at line 92, column 1
   |
92 | [tool.ruff]
   | ^^^^^^^^^^^
Unknown rule selector: `E999`

Changes by rule (14 rules affected)

code total + violation - violation + fix - fix
ANN101 35381 0 35381 0 0
ANN102 674 0 674 0 0
S603 454 227 227 0 0
E721 211 188 23 0 0
S610 22 22 0 0 0
S602 21 11 10 0 0
S604 16 8 8 0 0
S605 8 4 4 0 0
F811 8 8 0 0 0
PLR1701 3 0 3 0 0
D107 2 1 1 0 0
RUF100 2 2 0 0 0
NPY201 1 1 0 0 0
RUF024 1 1 0 0 0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+261 -256 violations, +0 -0 fixes in 12 projects; 1 project error; 37 projects unchanged)

PlasmaPy/PlasmaPy (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ src/plasmapy/diagnostics/langmuir.py:1396:23: NPY201 `np.trapz` will be removed in NumPy 2.0. Use `numpy.trapezoid` on NumPy 2.0, or ignore this warning on earlier versions.

apache/airflow (+171 -177 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ airflow/cli/commands/dag_command.py:309:14: S603 `subprocess` call: check for execution of untrusted input
- airflow/cli/commands/dag_command.py:309:31: S603 `subprocess` call: check for execution of untrusted input
+ airflow/cli/commands/info_command.py:199:18: S603 `subprocess` call: check for execution of untrusted input
- airflow/cli/commands/info_command.py:199:35: S603 `subprocess` call: check for execution of untrusted input
+ airflow/cli/commands/internal_api_command.py:166:17: S603 `subprocess` call: check for execution of untrusted input
- airflow/cli/commands/internal_api_command.py:166:34: S603 `subprocess` call: check for execution of untrusted input
... 297 additional changes omitted for rule S603
+ airflow/example_dags/example_kubernetes_executor.py:132:35: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
- airflow/example_dags/example_kubernetes_executor.py:132:45: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+ airflow/example_dags/example_kubernetes_executor.py:94:27: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
- airflow/example_dags/example_kubernetes_executor.py:94:37: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
+ airflow/providers/apache/beam/hooks/beam.py:575:25: S604 Function call with `shell=True` parameter identified, security issue
- airflow/providers/apache/beam/hooks/beam.py:577:13: S604 Function call with `shell=True` parameter identified, security issue
- airflow/providers/microsoft/azure/hooks/msgraph.py:327:12: PLR1701 [*] Merge `isinstance` calls: `isinstance(data, (BytesIO, bytes, str))`
- airflow/serialization/pydantic/dag.py:45:9: PLR1701 [*] Merge `isinstance` calls
- airflow/serialization/pydantic/taskinstance.py:70:8: PLR1701 [*] Merge `isinstance` calls: `isinstance(x, (BaseOperator, MappedOperator))`
- airflow/www/extensions/init_appbuilder.py:1:1: CPY001 Missing copyright notice at top of file
- airflow/www/fab_security/manager.py:1:1: CPY001 Missing copyright notice at top of file
+ dev/breeze/src/airflow_breeze/commands/kubernetes_commands.py:1082:13: S604 Function call with `shell=True` parameter identified, security issue
- dev/breeze/src/airflow_breeze/commands/kubernetes_commands.py:1091:17: S604 Function call with `shell=True` parameter identified, security issue
+ dev/breeze/src/airflow_breeze/commands/kubernetes_commands.py:1094:13: S604 Function call with `shell=True` parameter identified, security issue
- dev/breeze/src/airflow_breeze/commands/kubernetes_commands.py:1103:17: S604 Function call with `shell=True` parameter identified, security issue
... 11 additional changes omitted for rule S604
+ hatch_build.py:660:13: S602 `subprocess` call with `shell=True` identified, security issue
- hatch_build.py:660:59: S602 `subprocess` call with `shell=True` identified, security issue
+ hatch_build.py:673:13: S602 `subprocess` call with `shell=True` identified, security issue
- hatch_build.py:673:59: S602 `subprocess` call with `shell=True` identified, security issue
+ scripts/ci/pre_commit/ruff_format.py:26:1: S602 `subprocess` call with `shell=True` identified, security issue
- scripts/ci/pre_commit/ruff_format.py:26:33: S602 `subprocess` call with `shell=True` identified, security issue
... 11 additional changes omitted for rule S602
+ tests/dags/test_on_kill.py:44:13: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
- tests/dags/test_on_kill.py:44:23: S605 Starting a process with a shell: seems safe, but may be changed in the future; consider rewriting without `shell`
... 3 additional changes omitted for rule S605
- tests/providers/elasticsearch/log/elasticmock/fake_elasticsearch.py:1:1: CPY001 Missing copyright notice at top of file
... 318 additional changes omitted for project

bokeh/bokeh (+38 -38 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ examples/output/apis/server_document/flask_server.py:45:17: S603 `subprocess` call: check for execution of untrusted input
- examples/output/apis/server_document/flask_server.py:46:5: S603 `subprocess` call: check for execution of untrusted input
+ release/system.py:43:18: S602 `subprocess` call with `shell=True` identified, security issue
- release/system.py:43:34: S602 `subprocess` call with `shell=True` identified, security issue
- scripts/hooks/install.py:5:20: S603 `subprocess` call: check for execution of untrusted input
+ scripts/hooks/install.py:5:5: S603 `subprocess` call: check for execution of untrusted input
+ scripts/hooks/protect_branches.py:10:22: S603 `subprocess` call: check for execution of untrusted input
- scripts/hooks/protect_branches.py:10:26: S603 `subprocess` call: check for execution of untrusted input
... 69 additional changes omitted for rule S603
... 68 additional changes omitted for project

freedomofpress/securedrop (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ devops/scripts/verify-mo.py:116:16: S602 `subprocess` call with `shell=True` identified, security issue
+ devops/scripts/verify-mo.py:120:26: RUF100 [*] Unused `noqa` directive (unused: `S602`)

fronzbot/blinkpy (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ blinksync/blinksync.py:58:53: F811 Redefinition of unused `working` from line 55
+ blinksync/blinksync.py:81:53: F811 Redefinition of unused `working` from line 78

ibis-project/ibis (+3 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ ibis/expr/types/strings.py:1678:9: F811 Redefinition of unused `__mul__` from line 638
+ ibis/expr/types/strings.py:417:9: F811 Redefinition of unused `initcap` from line 412
+ ibis/tests/benchmarks/test_benchmarks.py:720:34: F811 Redefinition of unused `con` from line 705

prefecthq/prefect (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ src/prefect/cli/cloud/__init__.py:269:42: F811 Redefinition of unused `timeout_scope` from line 244

rotki/rotki (+4 -3 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ packaging/docker/entrypoint.py:144:11: S603 `subprocess` call: check for execution of untrusted input
- packaging/docker/entrypoint.py:144:28: S603 `subprocess` call: check for execution of untrusted input
- packaging/docker/entrypoint.py:166:26: S603 `subprocess` call: check for execution of untrusted input
+ packaging/docker/entrypoint.py:166:9: S603 `subprocess` call: check for execution of untrusted input
- packaging/docker/entrypoint.py:174:52: S602 `subprocess` call with `shell=True` seems safe, but may be changed in the future; consider rewriting without `shell`
+ packaging/docker/entrypoint.py:174:9: S602 `subprocess` call with `shell=True` seems safe, but may be changed in the future; consider rewriting without `shell`
+ rotkehlchen/tests/exchanges/test_kraken.py:654:58: F811 Redefinition of unused `cursor` from line 653

zulip/zulip (+37 -38 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ scripts/lib/check_rabbitmq_queue.py:143:26: S603 `subprocess` call: check for execution of untrusted input
- scripts/lib/check_rabbitmq_queue.py:144:9: S603 `subprocess` call: check for execution of untrusted input
+ scripts/lib/check_rabbitmq_queue.py:160:23: S603 `subprocess` call: check for execution of untrusted input
- scripts/lib/check_rabbitmq_queue.py:161:9: S603 `subprocess` call: check for execution of untrusted input
+ scripts/lib/hash_reqs.py:38:12: S603 `subprocess` call: check for execution of untrusted input
- scripts/lib/hash_reqs.py:38:36: S603 `subprocess` call: check for execution of untrusted input
... 69 additional changes omitted for rule S603
- zerver/lib/markdown/fenced_code.py:1:1: CPY001 Missing copyright notice at top of file
... 68 additional changes omitted for project

agronholm/anyio (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ tests/test_taskgroups.py:557:27: RUF100 [*] Unused `noqa` directive (unknown: `ASYNC101`)

python-trio/trio (+0 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview


encode/httpx (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

+ tests/client/test_auth.py:371:73: F811 Redefinition of unused `client` from line 366

demisto/content (error)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview

ruff failed
  Cause: Failed to parse /home/runner/work/ruff/ruff/checkouts/demisto:content/pyproject.toml
  Cause: TOML parse error at line 92, column 1
   |
92 | [tool.ruff]
   | ^^^^^^^^^^^
Unknown rule selector: `E999`

Changes by rule (9 rules affected)

code total + violation - violation + fix - fix
S603 454 227 227 0 0
S602 21 11 10 0 0
S604 16 8 8 0 0
S605 8 4 4 0 0
F811 8 8 0 0 0
CPY001 4 0 4 0 0
PLR1701 3 0 3 0 0
RUF100 2 2 0 0 0
NPY201 1 1 0 0 0

@dhruvmanila dhruvmanila added the cli Related to the command-line interface label Jun 19, 2024
@dhruvmanila dhruvmanila marked this pull request as ready for review June 19, 2024 01:33
@dhruvmanila dhruvmanila added the do-not-merge Do not merge this pull request label Jun 19, 2024
@dhruvmanila dhruvmanila force-pushed the dhruv/syntax-error-2 branch 3 times, most recently from aead4a1 to 119f7a5 Compare June 26, 2024 09:39
@dhruvmanila dhruvmanila removed the do-not-merge Do not merge this pull request label Jun 26, 2024
@dhruvmanila dhruvmanila added this to the v0.5.0 milestone Jun 26, 2024
Base automatically changed from dhruv/syntax-error-2 to ruff-0.5 June 27, 2024 02:21
@dhruvmanila dhruvmanila merged commit dabca7b into ruff-0.5 Jun 27, 2024
13 of 16 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/syntax-error-3 branch June 27, 2024 02:24
dhruvmanila added a commit that referenced this pull request Jun 27, 2024
## Summary

Follow-up to #11902 

This PR simplifies the `LinterResult` struct by avoiding the generic and
not store the `ParseError`.

This is possible because the callers already have access to the
`ParseError` via the `Parsed` output. This also means that we can
simplify the return type of `check_path` and avoid the generic `T` on
`LinterResult`.

## Test Plan

`cargo insta test`
Copy link

codspeed-hq bot commented Jun 27, 2024

CodSpeed Performance Report

Merging #11902 will degrade performances by 4.03%

Comparing dhruv/syntax-error-3 (0fe0d6d) with main (55f4812)

Summary

❌ 1 regressions
✅ 29 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main dhruv/syntax-error-3 Change
linter/all-rules[unicode/pypinyin.py] 2.1 ms 2.2 ms -4.03%

dhruvmanila added a commit that referenced this pull request Jun 27, 2024
## Summary

Follow-up to #11901 

This PR avoids displaying the syntax errors as log message now that the
`E999` diagnostic cannot be disabled.

For context on why this was added, refer to
#2505. Basically, we would allow
ignoring the syntax error diagnostic because certain syntax feature
weren't supported back then like `match` statement. And, if a user
ignored `E999`, Ruff would give no feedback if the source code contained
any syntax error. So, this log message was a way to indicate to the user
even if `E999` was disabled.

The current state of the parser is such that (a) it matches with the
latest grammar and (b) it's easy to add support for any new syntax.

**Note:** This PR doesn't remove the `DisplayParseError` struct because
it's still being used by the formatter.

## Test Plan

Update existing snapshots from the integration tests.
dhruvmanila added a commit that referenced this pull request Jun 27, 2024
## Summary

Follow-up to #11902 

This PR simplifies the `LinterResult` struct by avoiding the generic and
not store the `ParseError`.

This is possible because the callers already have access to the
`ParseError` via the `Parsed` output. This also means that we can
simplify the return type of `check_path` and avoid the generic `T` on
`LinterResult`.

## Test Plan

`cargo insta test`
dhruvmanila added a commit that referenced this pull request Jun 27, 2024
## Summary

Follow-up to #11902

This PR simplifies the `LinterResult` struct by avoiding the generic and
not store the `ParseError`.

This is possible because the callers already have access to the
`ParseError` via the `Parsed` output. This also means that we can
simplify the return type of `check_path` and avoid the generic `T` on
`LinterResult`.

## Test Plan

`cargo insta test`
@MichaReiser MichaReiser mentioned this pull request Jun 27, 2024
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
## Summary

Follow-up to #11901 

This PR avoids displaying the syntax errors as log message now that the
`E999` diagnostic cannot be disabled.

For context on why this was added, refer to
#2505. Basically, we would allow
ignoring the syntax error diagnostic because certain syntax feature
weren't supported back then like `match` statement. And, if a user
ignored `E999`, Ruff would give no feedback if the source code contained
any syntax error. So, this log message was a way to indicate to the user
even if `E999` was disabled.

The current state of the parser is such that (a) it matches with the
latest grammar and (b) it's easy to add support for any new syntax.

**Note:** This PR doesn't remove the `DisplayParseError` struct because
it's still being used by the formatter.

## Test Plan

Update existing snapshots from the integration tests.
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
## Summary

Follow-up to #11902

This PR simplifies the `LinterResult` struct by avoiding the generic and
not store the `ParseError`.

This is possible because the callers already have access to the
`ParseError` via the `Parsed` output. This also means that we can
simplify the return type of `check_path` and avoid the generic `T` on
`LinterResult`.

## Test Plan

`cargo insta test`
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
## Summary

Follow-up to #11901 

This PR avoids displaying the syntax errors as log message now that the
`E999` diagnostic cannot be disabled.

For context on why this was added, refer to
#2505. Basically, we would allow
ignoring the syntax error diagnostic because certain syntax feature
weren't supported back then like `match` statement. And, if a user
ignored `E999`, Ruff would give no feedback if the source code contained
any syntax error. So, this log message was a way to indicate to the user
even if `E999` was disabled.

The current state of the parser is such that (a) it matches with the
latest grammar and (b) it's easy to add support for any new syntax.

**Note:** This PR doesn't remove the `DisplayParseError` struct because
it's still being used by the formatter.

## Test Plan

Update existing snapshots from the integration tests.
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
## Summary

Follow-up to #11902

This PR simplifies the `LinterResult` struct by avoiding the generic and
not store the `ParseError`.

This is possible because the callers already have access to the
`ParseError` via the `Parsed` output. This also means that we can
simplify the return type of `check_path` and avoid the generic `T` on
`LinterResult`.

## Test Plan

`cargo insta test`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command-line interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants