Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should throw should allow for assignments to throw exceptions #387

Closed
LeoColman opened this issue Jul 27, 2018 · 10 comments
Closed

Should throw should allow for assignments to throw exceptions #387

LeoColman opened this issue Jul 27, 2018 · 10 comments
Assignees
Labels
bug 🐛 Issues that report a problem or error in the code.

Comments

@LeoColman
Copy link
Member

I expect this syntax to be possible, as the assignment throws an exception:

class Playground : FreeSpec() {
    
    class Foo {
        var bar = "bar"
        set(value) {
            throw Exception()
        }
    }
    
    init {
        "Should throw" {
            val foo = Foo()
            
            shouldThrow<Exception> { 
                foo.bar = "baz"
            }
        }
    }
}
@sksamuel sksamuel self-assigned this Jul 27, 2018
@sksamuel sksamuel added the bug 🐛 Issues that report a problem or error in the code. label Jul 27, 2018
@sksamuel
Copy link
Member

Ok so the issue is that an assignment is not an expression, and the shouldThrow excepts a function that returns a value.

We can change the shouldThrow definition to say the function returns a unit, but then you get warnings if your function does return a value.

I'm not sure if it's possible to get both to work, if not, I would have to add another function like shouldThrowUnit for use cases above. What do you think ?

@LeoColman
Copy link
Member Author

LeoColman commented Jul 27, 2018

A workaround, current is to do something like

shouldThrow<Exception> {
foo.bar = "baz"
""   //This will be returned
}

I'm cooking a Pull Request for this:
https://github.com/kerooker/kotlintest

I also put this in discussion in jetbrains youtrack:
https://youtrack.jetbrains.com/issue/KT-25773

@LeoColman
Copy link
Member Author

The only problem in my branch right now is that you can't have both the functions in the same class. We'll have to put them in another namespace, as they're considered the same in this class, but it might work

@sksamuel
Copy link
Member

The function would have to be called shouldThrowUnit { } so that it has a different name. Not elegant but would work.

@LeoColman
Copy link
Member Author

It's not a problem to have the same name, they just must be in different namespaces

@sksamuel
Copy link
Member

Is that more confusing or less? If you tried to use both in the same file, one would just not compile, and you'd have to know to manually add another import ?

@LeoColman
Copy link
Member Author

And one can be an overload of the other. For example, I created the workaround in my application:

inline fun <reified T: Throwable> shouldThrow(thick: () -> Unit) : T {
        return io.kotlintest.shouldThrow(thick)
    }

This can be in kotlintest too, but must be in different packages. I'm not usre if it's less elegant than shouldThrowUnit

@LeoColman
Copy link
Member Author

That may be more confusing, indeed

@sksamuel
Copy link
Member

Both are not ideal, perhaps there's a trick one can do that one of the Jetbrains team knows about @udalov

sksamuel added a commit that referenced this issue Aug 9, 2018
@sksamuel
Copy link
Member

sksamuel commented Aug 9, 2018

In the end I've added shouldThrowUnit until this anomaly is fixed by Jetbrains.

@sksamuel sksamuel closed this as completed Aug 9, 2018
sksamuel added a commit that referenced this issue Sep 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Issues that report a problem or error in the code.
Projects
None yet
Development

No branches or pull requests

2 participants