Migrate accessibility rubocop rule LinkHref from dotcom to erblint-github

[bolonio]

Context

The motivation of rubocop-github is to open-source our accessibility rubocop rules so non-GitHub people can benefit from them, have a space to provide comprehensive rule documentation, and also allow rules to be shared between Rails projects.

This PR migrates the accessibility rule LinkHref from dotcom to rubocop-github

Related Issue

• https://github.com/github/accessibility/issues/1430

[bolonio]

I think this logic is wrong.

When using <%= link_to 'Go to GitHub', 'https://github.com/' %> then args is [s(:str, "Go to GitHub"), s(:str, "https://github.com/")] meaning that args.first is the content of the link, and the the second argument args[1] is the content of the href attribute.

@khiga8 what do you think? Shall we change the logic to check the second argument?

[owenniblock]

Does the test fail?

Maybe we should also add a test like this?

 def test_link_href_hash_offense
offenses = erb_investigate cop, <<-ERB, "app/views/products/index.html.erb"
<%= link_to 'Go to GitHub', '#' %>
ERB
assert_equal 1, offenses.count
assert_equal "Links should go somewhere, you probably want to use a `<button>` instead.", offenses[0].message
end

Seems like our code should pass on all three of the tests including this one?

[khiga8]

ooh yeah this logic does seem off. nice catch!

it is checking if the label is # rather than the href. I was looking through link_to docs and it looks like there's various formats supported like:

link_to "GitHub", "#"
link_to nil, "http://github.com" link_to @profile do Mona's profile
end 

We can iterate on this linter in the future but maybe for now we just check if the second argument exists, check if it's a string, check if it's # and use the test case @owenniblock proposed?

So maybe something like:

ifargs.length > 1 && args[1].type == :str && args[1].children.first == "#"

[khiga8]

as a future follow-up, we could add support for block syntax like:

link_to '#' do Mona's profile
end 

[khiga8]

Should this be in test/accessibility/?

[bolonio]

I didn’t put the tests inside a subfolder because bundle exec rake doesn’t find them. It only executes the test on the test folder, no subfolders