Correcting go.mod and go.sum to generated files instead of AMPL.

[leighlondon]

Correcting go.mod and go.sum to Go.

Description

Go's new "modules" feature uses some additional filenames - go.mod and go.sum. The go.mod file is misclassified as "AMPL" due to fallthrough in the heuristics.

I've added a new go.mod sample file and adjusted the heuristics to try get these module files classified as Go.

Checklist:

• I am associating a language with a new file extension.

  • The new extension is used in hundreds of repositories on GitHub.com
    • Search results for each extension:
      • https://github.com/search?utf8=%E2%9C%93&type=Code&ref=searchresults&q=extension%3AFOOBAR+KEYWORDS+NOT+nothack
  • I have included a real-world usage sample for all extensions added in this PR:
    • Sample source(s):
      • [URL to each sample source, if applicable]
    • Sample license(s):
  • I have included a change to the heuristics to distinguish my language from others using the same extension.

• I am adding a new language.

  • The extension of the new language is used in hundreds of repositories on GitHub.com.
    • Search results for each extension:
      • https://github.com/search?utf8=%E2%9C%93&type=Code&ref=searchresults&q=extension%3AFOOBAR+KEYWORDS+NOT+nothack
  • I have included a real-world usage sample for all extensions added in this PR:
    • Sample source(s):
      • [URL to each sample source, if applicable]
    • Sample license(s):
  • I have included a syntax highlighting grammar: https://github-lightshow.herokuapp.com/
  • I have updated the heuristics to distinguish my language from others using the same extension.

• I am fixing a misclassified language

  • I have included a new sample for the misclassified language:
    • Sample source(s):
      • go.mod example added
    • Sample license(s):
  • I have included a change to the heuristics to distinguish my language from others using the same extension.

• I am changing the source of a syntax highlighting grammar

  • Old: https://github-lightshow.herokuapp.com/
  • New: https://github-lightshow.herokuapp.com/

• I am updating a grammar submodule

• I am adding new or changing current functionality

  • I have added or updated the tests for the new or changed functionality.

[lildude]

To save you time: we don't need a heuristic for explicitly named files. The analysis will never get to the heuristic 😄

[leighlondon]

Oh, even easier then!

[pchaigno]

Thanks for your contribution!

I've left two comments to address inline 👇

[leighlondon]

Adding the samples broke the tests so I've moved them to the right location now.

[leighlondon]

@pchaigno is there a way to "unlanguage" a certain file then? If it's not necessarily Go (since it's not), is there a way for us to say that it's not anything else as well?

[pchaigno]

is there a way to "unlanguage" a certain file then?

There's not. It's a known limitation of Linguist which we've discussed several times, but haven't found the time to address.

Maybe we can address this another way: Are these files generated or are they written by humans? Are go.mod files closer to some other language? It looks like they have structure (require, module, etc.); how are they processed?

[leighlondon]

They're "generated" files, and rarely (but not never) edited by humans - specifically, they're generated by the Go binary, using go mod tidy and so on. There's some info with some details about the mod file in the golang/go wiki but not specifically about the parsing of it, which would take a bit of a dive through the code to write up a format for a language bundle.

The closest analogy is probably to JavaScript's package.json and package-lock.json - dependency metadata which is linked specifically to the language (though go.mod isn't JSON, unfortunately, and go.sum is just a list of cryptographic hashes).

I had a look at the code in generated.rb and it says that they would be "suppressed in diffs" if generated, is that specifically for things like GitHub pull requests?

[pchaigno]

They're "generated" files, and rarely (but not never) edited by humans - specifically, they're generated by the Go binary, using go mod tidy and so on.

Maybe we should add them to generated.rb? go.mod would still be recognized as AMPL, but at least it wouldn't count in language statistics. What do you think?

I had a look at the code in generated.rb and it says that they would be "suppressed in diffs" if generated, is that specifically for things like GitHub pull requests?

Yes, it means they won't show up in diffs across GitHub.com (PRs, branch comparisons, etc.). That won't affect the git diff command output if that was your concern.

[leighlondon]

The main (only?) thing I was trying to do was just to stop repos and statistics showing AMPL as a language in that upper bar.

I think switching to generated would cover that from what you've mentioned, which would be great - I should be able to make those changes and push a new update.

[leighlondon]

Do you want the explicit "filenames" section removed from the language specification? From my understanding that is not required if it's generated?

The samples are used in the tests so I've left them in (but can remove them if needed).

Thanks for the help! 👍

[pchaigno]

Do you want the explicit "filenames" section removed from the language specification? From my understanding that is not required if it's generated?

I'd rather we remove them.

The samples are used in the tests so I've left them in (but can remove them if needed).

If we remove the filenames in languages.yml, we'll also need to remove the samples. They're not used in the tests anyway since you don't need to read the content of files, only to check their name.

[leighlondon]

Changes were made, but the Travis build is still going at 30 minutes... checking all the builds shows that Travis thinks they all passed (Done. Your build exited with 0.) but it won't end. As far as I can tell it might be a Travis error, but I'm not sure, I didn't exactly change a lot.

[pchaigno]

Looks like a bug in Travis CI (maybe due to the Daylight saving time change in Europe). I tried to restart the builds but there seems to be the same problem again. I'll try again this evening or tomorrow if it doesn't resolve itself in the meantime.

[leighlondon]

Finally passed now! 🎉

[pchaigno]

LGTM!

@leighlondon Thanks for your contribution and your patience!

[leighlondon]

Thanks for the help and being so patient with me @pchaigno 😄

[lildude]

LGTM Thanks for the contribution and welcome to Linguist.

[myitcv]

@leighlondon @pchaigno - changes to go.mod and go.sum files represent something that should be reviewed as part of a PR, so treating them as generated files is not correct to my mind.

The fact that go.mod largely represents the result of a tool having modified the file is a function of good tooling as opposed to the file by definition being generated.

Changes to go.sum are of interest because it highlights transitive dependencies being added to/removed from your module.

[mvdan]

I very much agree with @myitcv - even if one should never edit go.sum manually, both files are very important during reviews. And go.mod has its own format precisely to make it readable and writable by humans.

[pchaigno]

@lildude Should we revert this? How did we handle this kind of issues in the past?

[myitcv]

@lildude @pchaigno I've pushed up #4333 in case you want to move forward instead of reverting.

[leighlondon]

Could be reverted or the original suggestion could be applied to update heuristics by filename match- all up to you guys @pchaigno and @lildude. Either way is fine. :)

[lildude]

@lildude Should we revert this? How did we handle this kind of issues in the past?

If the general consensus from the Go community is that these should not be marked as generated and thus should be reviewed, then lets (@pchaigno, @Alhadis or me) go ahead and click the Revert button. If we could get some referenceable guidelines, that would be an added bonus so we can avoid a debate about this in future.

[mvdan]

You could have a look at https://github.com/golang/go/wiki/Modules#gomod or https://github.com/golang/go/wiki/Modules#faqs--gomod-and-gosum.

There's no specific entry in the FAQ about whether these files should be hidden during code reviews. But no part of the document says that they should be ignored or treated like generated files.

[pchaigno]

@lildude I'm afraid I don't have the revert button.

[myitcv]

@lildude

If we could get some referenceable guidelines, that would be an added bonus so we can avoid a debate about this in future.

Just to add to @mvdan's comments, I would also suggest that a question to https://groups.google.com/forum/#!forum/golang-nuts would also be an excellent way of getting an "official" answer.

[myitcv]

@lildude - please can we get an update on this?

[lildude]

@myitcv sorry, I'm not had a chance to do anything remotely Linguist-based this week and not sure I'll have the time next week. I'd really appreciate it if you could pose this question to the golang-nuts forum to get an idea of which way peeps prefer things - keep as generated or show in diff, both of which can easily be overridden within .gitattributes.

[myitcv]

@lildude - there is already a convention for indicating Go-related files are generated:

https://golang.org/s/generatedcode

Add that to the comments above and the wiki link, I don't think this is case of what default people prefer. By default, go.{mod,sum} files should not be marked as generated.

As you say, if people prefer something other than the default (which I can't quite comprehend, because that would mean they prefer not to see changes in the (transitive) dependencies of their projects) they can easily do so.

cc @bcmills from the Go team in case there is anything to add/amend to the above.

[lildude]

Ah, that's interesting and very useful. Thanks @myitcv. I think we can definitely rely on that as an indicator these files shouldn't be marked as generated (yet) as Go isn't adding the standard text to indicate they should be.

That all said, we can't simply revert this PR as that will take us back to these files not being associated with Go at all leading the behaviour this PR originally attempted to address.

Update: ignore that.

Named file entries would need to added too. I've got a few free mins now so will do it now.

[lildude]

Booo!!!

New PR it is then.