configlet tree: crashes on "null"

[rpottsoh]

Update: tree is crashing when it comes across "null" when it is expecting null.

configlet tree . is crashing when I run it against the CSharp track. It is working fine against Delphi and Ruby.

Here is the output I am receiving when I run it against the CSharp track:

C:\projects.git\Exercism\csharp>configlet tree .
C#
==
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x2c pc=0x667f17]

goroutine 1 [running]:
github.com/exercism/configlet/cmd.treeTrack(0x11c40740, 0x1, 0x8a1b20, 0x11c34690)
        /home/wilken/Development/golang/src/github.com/exercism/configlet/cmd/tree.go:235 +0x717
github.com/exercism/configlet/cmd.runTree(0x8a1b20, 0x11c34690, 0x1, 0x1)
        /home/wilken/Development/golang/src/github.com/exercism/configlet/cmd/tree.go:110 +0x50
github.com/exercism/configlet/vendor/github.com/spf13/cobra.(*Command).execute(0x8a1b20, 0x11c34680, 0x1, 0x1, 0x8a1b20, 0x11c34680)
        /home/wilken/Development/golang/src/github.com/exercism/configlet/vendor/github.com/spf13/cobra/command.go:653 +0x1dc
github.com/exercism/configlet/vendor/github.com/spf13/cobra.(*Command).ExecuteC(0x8a1a00, 0x11c48000, 0x11c48000, 0x403f73)
        /home/wilken/Development/golang/src/github.com/exercism/configlet/vendor/github.com/spf13/cobra/command.go:728 +0x264
github.com/exercism/configlet/vendor/github.com/spf13/cobra.(*Command).Execute(0x8a1a00, 0x1, 0x11c3a6c0)
        /home/wilken/Development/golang/src/github.com/exercism/configlet/vendor/github.com/spf13/cobra/command.go:687 +0x21
github.com/exercism/configlet/cmd.Execute()
        /home/wilken/Development/golang/src/github.com/exercism/configlet/cmd/root.go:42 +0x27
main.main()
        /home/wilken/Development/golang/src/github.com/exercism/configlet/main.go:6 +0x17

Configlet lint is not reporting any problems with CSharp. I am using the 32 bit Windows version of Configlet. Let me know if you need anymore information.

[nywilken]

@rpottosh thanks for reporting this issue. I have not confirmed this, but it looks like configlet is expecting an instance of an exercise and is actually getting nil on line 235. Taking a quick look at the config.json for csharp I see the two-fer exercise has a string value of ”null” for unlocked_by - It should be null. https://github.com/exercism/csharp/blob/master/config.json#L67

Could you try sanitizing the json, and change the value to null (manually) and try running tree again. Configlet, doesn’t validate unlocked_by yet so this may be the issue. Please let me know.

[rpottsoh]

@nywilken I have changed "null" to null on line 67 of config.json for two-fer. This resolves the problem and configlet tree . works fine.

Should configlet lint catch this, and/or Travis CI? Should I bother posting an issue?

I am going to go ahead and submit a PR to CSharp.

[tleen]

It should probably be caught in a few places. I purposely assumed a well-formed config when tree executes. Perhaps there should be a few sanity checks + warnings in there.

Definitely lintable and a worthy issue, this one could serve as that issue. Or another one, but leave this one open and I will at least fix the issue in tree to close it?

[rpottsoh]

@nywilken said:

Configlet, doesn’t validate unlocked_by yet

Sounds like @nywilken may be planning to beef up tree.

[nywilken]

A little late on responses - family vacation. @rpottsoh thanks again for reporting and fixing this within the csharp track. As @tleen said already, this issue can serve as the bug.

@tleen I haven't had a chance to look at your updated PRs just yet so I apologize if this has already been addressed. While looking through the code I saw that we don't actually check that we get a valid item from slugByExercise https://github.com/exercism/configlet/blob/master/cmd/tree.go#L234

I would expect something like

if parent, ok := slugToExercise[e.UnlockedBy]; ok {
   parent.childSlugs = append(parent.childSlugs, e.Slug)
}

[nywilken]

Sounds like @nywilken may be planning to beef up tree.

There is an open issue/request for validating unlocked_by and core when it comes to configlet lint. I suspect that functionality would've caught this issue.

[tleen]

@nywilken yep I believe it would have but it's ok for tree to do some sanity checking too. Updating lint would be the second part of a fix on this.

if parent, ok := slugToExercise[e.UnlockedBy]; ok {
   parent.childSlugs = append(parent.childSlugs, e.Slug)
}

More or less that's the fix + some scopy stuff and a warning about the config.