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

Please add rdjson output support #8655

Closed
Keramblock opened this issue Nov 13, 2023 · 8 comments
Closed

Please add rdjson output support #8655

Keramblock opened this issue Nov 13, 2023 · 8 comments
Labels
configuration Related to settings and configuration help wanted Contributions especially welcome

Comments

@Keramblock
Copy link

Keramblock commented Nov 13, 2023

Hi, it would be nice, if ruff could support rdjson: https://github.com/reviewdog/reviewdog/tree/master/proto/rdf#rdjson both for linter and format mode(if possible)

Why? because it allows to provide problem description to PR and suggest fix at the same time using reviewdog: https://github.com/reviewdog/reviewdog

Here is an example of results of such pipeline(rdjson output by eslint):
eslint rdjson parsed

Useful links by @eremeevfd:
jsonschema: https://github.com/reviewdog/reviewdog/blob/master/proto/rdf/jsonschema/DiagnosticResult.jsonschema
actual jq parser for Hado-Lint: https://github.com/reviewdog/action-hadolint/blob/master/to-rdjson.jq

@charliermarsh charliermarsh added configuration Related to settings and configuration help wanted Contributions especially welcome labels Nov 13, 2023
@charliermarsh
Copy link
Member

No objection to including this, PR welcome if anyone is up for it!

@eremeevfd
Copy link

@eremeevfd
Copy link

# Convert hadolint JSON output to Reviewdog Diagnostic Format (rdjson)
# https://github.com/reviewdog/reviewdog/blob/f577bd4b56e5973796eb375b4205e89bce214bd9/proto/rdf/reviewdog.proto
{
  source: {
    name: "ruff",
    url: "https://github.com/astral-sh/ruff"
  },
  diagnostics: . | map({
    message: .message,
    code: {
      value: .code,
      url: .url,
    },
    location: {
      path: .filename,
      range: {
        start: {
          line: .location.row,
          column: .location.column
        },
        end: {
            line: .end_location.row,
            column: .end_location.column
        }
      }
    },
    suggestions: (.fix.edits // {}) | map(
        {
            range: {
                start: {
                    line: .location.row,
                    column: .location.column
                },
                end: {
                    line: .end_location.row,
                    column: .end_location.column
                }
            },
            text: .content
        }
    ),
    severity: "WARNING"
  })
}

I've managed to apply this JQ to ruff check output:

ruff check ./ --output-format=json | jq -f "./to-rdjson.jq

And get this:

{
  "source": {
    "name": "ruff",
    "url": "https://github.com/astral-sh/ruff"
  },
  "diagnostics": [
    {
      "message": "Import block is un-sorted or un-formatted",
      "code": {
        "value": "I001",
        "url": "https://docs.astral.sh/ruff/rules/unsorted-imports"
      },
      "location": {
        "path": "/Users/f.eremeev/hautaiou/saas-backend-core/source/assessor/views/b2b/assessments.py",
        "range": {
          "start": {
            "line": 1,
            "column": 1
          },
          "end": {
            "line": 38,
            "column": 1
          }
        }
      },
      "suggestions": [
        {
          "range": {
            "start": {
              "line": 1,
              "column": 1
            },
            "end": {
              "line": 38,
              "column": 1
            }
          },
          "text": "import uuid\n\nfrom django.db.models.manager import BaseManager\nfrom django.http import StreamingHttpResponse\nfrom drf_spectacular.utils import (\n    OpenApiExample,\n    OpenApiResponse,\n    OpenApiTypes,\n    extend_schema,\n    extend_schema_view,\n)\nfrom rest_framework import exceptions, parsers, serializers, viewsets\nfrom rest_framework.decorators import action\nfrom rest_framework.request import Request\nfrom rest_framework.response import Response\nfrom rest_framework.settings import api_settings\nfrom rest_framework_csv.renderers import CSVRenderer\n\nfrom assessor.models import Assessment\nfrom assessor.serializers.assessments import (\n    AssessmentCreate,\n    AssessmentRead,\n    AssessmentUpdate,\n    UploadImageComparisonPairs,\n)\nfrom assessor.services.assessments import (\n    ExportCsv,\n    ImageComparisonPairs,\n    ImageComparisonPairsExample,\n    SetImageComparisonPairsFromCSV,\n)\nfrom core_v2.models import Company, Image\nfrom drf_rw_serializers import generics\n\n"
        }
      ],
      "severity": "WARNING"
    },
    {
      "message": "Missing return type annotation for public function `get_queryset`",
      "code": {
        "value": "ANN201",
        "url": "https://docs.astral.sh/ruff/rules/missing-return-type-undocumented-public-function"
      },
      "location": {
        "path": "/Users/f.eremeev/hautaiou/saas-backend-core/source/assessor/views/b2b/assessments.py",
        "range": {
          "start": {
            "line": 73,
            "column": 9
          },
          "end": {
            "line": 73,
            "column": 21
          }
        }
      },
      "suggestions": [],
      "severity": "WARNING"
    }
  ]
}

@Keramblock
Copy link
Author

But this will not work with format, AFAIK

@eremeevfd
Copy link

Yep, format doesn't have any output formats except main diff

charliermarsh pushed a commit that referenced this issue Jun 2, 2024
## Summary

Implement support for RDJson output for `ruff check`, as requested in
#8655.

## Test Plan

Tested using a snapshot test. Same approach as for e.g. the JSON output
formatter.

## Additional info

I tried to keep the implementation close to the JSON implementation.

I had to deviate a bit to make the `suggestions` key work: If there are
no suggestions, then setting `suggestions` to `null` is invalid
according to the JSONSchema. Therefore, I opted for a slightly more
complex implementation, that skips the `suggestions` key entirely if
there are no fixes available for the given diagnostic. Maybe it would
have been easier to set `"suggestions": []`, but I ended up doing it
this way.

I didn't consider notebooks, as I _think_ that RDJson doesn't work with
notebooks. This should be confirmed, and if so, there should be some
form of warning or error emitted when trying to output diagnostics for a
notebook.

I also didn't consider `ruff format`, as this comment:
#8655 (comment)
suggests that that wouldn't be compatible.

I'm new to Rust, any feedback is appreciated. 🙂 I
implemented this in order to have a productive rainy saturday afternoon,
I'm not knowledgeable about RDJson beyond the sources linked in the
issue.
@tobb10001
Copy link
Contributor

In #11682 I implemented RDJson formatting for ruff check. I didn't address anything regarding formatting.

@charliermarsh
Copy link
Member

👍 For me this is enough to close -- we don't support alternate output formats for format at all right now.

@Keramblock
Copy link
Author

@tobb10001 thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Related to settings and configuration help wanted Contributions especially welcome
Projects
None yet
Development

No branches or pull requests

4 participants