Skip to content

fix(report-converter): Support null column in eslint reports #4497

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

Merged
merged 1 commit into from
Mar 20, 2025

Conversation

SweetVishnya
Copy link
Contributor

ESLint reports may contain bugs with "column": null. Thus, report-converter tool raises an exception:

TypeError: int() argument must be a string, a bytes-like object or a number, not 'NoneType'

We set column number to 0 in such cases.

ESLint reports may contain bugs with "column": null. Thus,
report-converter tool raises an exception:

TypeError: int() argument must be a string, a bytes-like object or a
number, not 'NoneType'

We set column number to 0 in such cases.
@SweetVishnya
Copy link
Contributor Author

@bruntib, can you take a look, please?

Copy link
Contributor

@bruntib bruntib left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! I will approve this change, but could you, please, show an ESLint report where the column number is not available?
This question has two motivations:

  • I would just like to make sure that this fix won't hide a potential parser bug of the report converter.
  • I would like to test that if it causes any rendering issue in CodeChecker GUI to place a message bubble to a non-existing column number.

Thank you!

@SweetVishnya
Copy link
Contributor Author

@bruntib, here are some examples:

      {
        "ruleId": "jsdoc/newline-after-description",
        "severity": 2,
        "message": "There must be no newline after the description of the JSDoc block.",
        "line": 38,
        "column": null,
        "nodeType": "Block",
        "endLine": 38,
        "endColumn": null
      },
      {
        "ruleId": "jsdoc/require-description-complete-sentence",
        "severity": 2,
        "message": "Sentence must end with a period.",
        "line": 116,
        "column": null,
        "nodeType": "Block",
        "endLine": 116,
        "endColumn": null
      },
      {
        "ruleId": "jsdoc/check-types",
        "severity": 2,
        "message": "Invalid JSDoc @param \"opt_propertyBag\" type \"Object\"; prefer: \"object\".",
        "line": 233,
        "column": null,
        "nodeType": "Block",
        "endLine": 233,
        "endColumn": null
      },
      {
        "ruleId": "jsdoc/check-tag-names",
        "severity": 2,
        "message": "Invalid JSDoc tag (preference). Replace \"constructor\" JSDoc tag with \"class\".",
        "line": 234,
        "column": null,
        "nodeType": "Block",
        "endLine": 234,
        "endColumn": null
      },

@bruntib bruntib merged commit 10f3b59 into Ericsson:master Mar 20, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants