Skip to content

Spec inconsistency around trace/location/other_locations (or maybe I'm just confused) #56

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

Closed
srenatus opened this issue Jun 6, 2017 · 3 comments
Assignees

Comments

@srenatus
Copy link

srenatus commented Jun 6, 2017

Issues claims that an example output would look like this:

{
  "type": "issue",
  "check_name": "Bug Risk/Unused Variable",
  "description": "Unused local variable `foo`",
  "content": Content,
  "categories": ["Complexity"],
  "location": Location,
  "other_locations": [Location],  # <<< note that
  "remediation_points": 50000,
  "severity": Severity,
  "fingerprint": "abcd1234"
}

while the text below explains trace (which isn't part of the JSON blob):

trace -- Optional. A Trace object representing other interesting source code locations related to this issue.

Further down we find an explanation of the location key, that's explaining that (I've highlighted what I'm confused about):

Locations refer to ranges of a source code file. A Location contains a path, a source range, (expressed as lines or positions), and an optional array of other_locations.

What is correct, where does other_locations go? 😃

Thanks,
Stephan

@ABaldwinHunter
Copy link
Contributor

Hi Stephan,

Thanks for your question!

The location object usually looks like the examples given here:

  • there's always a path
  • the source code position can be expressed with begin / end lines, or with line and column positions
{
  "path": "path/to/file.css",
  "lines": {
    "begin": 13,
    "end": 14
  }
}
{
  "path": "path/to/file.css",
  "positions": {
    "begin": {
      "line": 3,
      "column": 10
    },
    "end": {
      "line": 4,
      "column": 12
    }
  }
}

The part you noted about other_locations seems confusing to me - as I think the example output is correct- other_locations should be a top level key (that's optional).

Historically, it's been used with our duplication engine to reference the locations of similar code.

Going to go ahead and open a PR to clarify.

Thanks!

@srenatus
Copy link
Author

srenatus commented Jun 6, 2017

@ABaldwinHunter Thank you 😃

ABaldwinHunter pushed a commit that referenced this issue Jun 6, 2017
* Clarify usage of other_locations key

Addresses #56
@ABaldwinHunter
Copy link
Contributor

@srenatus updated documentation in #57.

Thanks for the heads up.

Going to close this issue for now but please feel free to reopen if anything related comes up.

ches added a commit to ches/platform that referenced this issue Sep 18, 2021
It appears that codeclimate#20 intended to remove `other_locations` as superseded
by `trace`, but it left the old entry (only) in the example JSON object.
An expanded section describing `other_locations` was put back in codeclimate#57,
that issue thread asserts that it is still valid and in use by Code
Climate's duplication engine.

These two fields certainly seem functionally redundant, hence codeclimate#56.

Under assumption that both are used/accepted in implementations of the
current version of the spec, this change restores presence of both in
the reference but proposes deprecating `other_locations` and hence
de-emphasizes its documentation.
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

No branches or pull requests

2 participants