Possible bug in relative link filter due to invalid conditional
Summary
When we were working on improving our static analysis, we encountered a problem with ternary operator and conditional that may not work that way author expected it to work.
See comments on diffs for MR https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4356#note_12164575
Description
Original code, after fixing multiline ternary offense detected by Rubocop looked like:
context[:commit].try(:id) ||
ref ? repository.commit(ref).try(:sha) : repository.head_commit.sha
which is not the same like:
context[:commit].try(:id) ||
(ref ? repository.commit(ref).try(:sha) : repository.head_commit.sha)
because conditional ||
takes precedence over ternary operator ? ... :
.
What is interesting is that tests are passing for the first version, but not for the second version, which may indicate that tests were written after implementation had been made.
The previous version of this code, before refactoring in https://gitlab.com/gitlab-org/gitlab-ce/commit/b3276661f7c5077c1254b5463bb16c5e46b01abf by @jirutka was:
if context[:commit]
context[:commit].id
elsif ref
repository.commit(ref).try(:sha)
else
repository.head_commit.sha
end
which may indicate that the new implementation is wrong, but because tests still pass, something is wrong there.
This needs a little more investigation. /cc @rspeicher