Implement anti-duplicate parsing for config.tools

[OverHash]

During deserialization, ensures that there are no duplicate entries in config.tools. By default, serde will overwrite duplicate entries when deserializing to a HashMap<K, V>.

In almost all cases, a user would be incorrectly doing so accidentally, so we now throw an error when parsing the configuration file.

A unit test has been implemented to test the logic of this deserialization for duplicate tool entries.

tool = { github = "user/repo", version = "0.1.0" }
tool = { github = "user2/repo2", version = "0.2.0" }

will error with

duplicate tool `tool` at line 1 column 1

Closes #63

[OverHash]

The cleanest way I found to do this was to create a wrapper struct around the tools item in the Config struct and then implement a serde::Deserialize on it. This gives slightly less liberty around the error message, but it does still show the duplicate key name which is the most critical aspect. Confusingly, the line 1 column 1 seems to exist no matter where the duplicate key is.

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[oltrep]

Would you mind adding a test case to document the issue you're having with the line/column number being always 1?

I would like to see something like these cases:

tool_a = { github = "user/a", version = "0.1.0" }
tool_b = { github = "user/b", version = "0.1.0" }
tool_a = { github = "user/c", version = "0.1.0" }

tool_b = { github = "user/b", version = "0.1.0" }
tool_a = { github = "user/a", version = "0.1.0" }
tool_a = { github = "user/c", version = "0.1.0" }

[OverHash]

Added these test cases now.

❯ cat foreman.toml


[tools]
# oops!
selene = { source = "rojo-rbx/rojo", version = "=7.1.1" }

selene = { source = "Kampfkarren/selene", version = "=0.19.1" }
❯ selene --version
unable to parse Foreman configuration file (at ~/project/foreman.toml): duplicate tool name `selene` found for key `tools` at line 3 column 1

A Foreman configuration file looks like this:

[tools] # list the tools you want to install under this header

# each tool is on its own line, the tool name is on the left
# side of `=` and the right side tells Foreman where to find
# it and which version to download
tool_name = { github = "user/repository-name", version = "1.0.0" }

# tools hosted on gitlab follows the same structure, except
# `github` is replaced with `gitlab`

# Examples:
stylua = { github = "JohnnyMorganz/StyLua", version = "0.11.3" }
darklua = { gitlab = "seaofvoices/darklua", version = "0.7.0" }

So I think it's not really an issue, it's just that the line serde points to is where [tools] is in the toml, rather than where the duplicate tool is. Unsure if that is a big enough issue to warrant looking in to.

[oltrep]

If you want, I think a neat tiny little improvement would be to make this field private and add a public method like pub fn tools(&self) -> impl Iterator<Item = &(String, ToolSpec)>

[OverHash]

Since some external files (like src/main.rs) want to perform methods like tools.get, I think it would probably be best to just do

pub fn tools(&self) -> &HashMap<String, ToolSpec> {
    &self.tools.0
}

which would leave us to have to create a tools_mut as well (as ConfigFile::fill_from needs it), or we could leave it the current way it is (implement Deref and DerefMut).

[OverHash]

Looks like CI is failing as it's using Rust 1.53 which is before named parameters was implemented in 1.58

[OverHash]

Just fixed conflicts with master branch, so should be good to have a final review now.

I can bump CI in this PR (or another PR) to Rust 1.58 to support named parameters, or if you want to maintain the older Rust version, can change the code around to not use named parameters.

[oltrep]

Just fixed conflicts with master branch, so should be good to have a final review now.

I can bump CI in this PR (or another PR) to Rust 1.58 to support named parameters, or if you want to maintain the older Rust version, can change the code around to not use named parameters.

Thanks for the follow up! Yes let's bump to Rust 1.58 🙂 I'll take a final look after that and merge that in, if you could add an entry to the changelog that would be great