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

Missing check for @Duck void procedures #144

Open
jlmaddox opened this issue Mar 23, 2016 · 5 comments
Open

Missing check for @Duck void procedures #144

jlmaddox opened this issue Mar 23, 2016 · 5 comments

Comments

@jlmaddox
Copy link
Collaborator

There is at least one missing check in proc right now, but also possibly others. We need to look for places with missing checks and then create those checks. As an example, using @Duck on a procedure returning void is not presently checked and breaks the processor.

Additionally, it would be a good idea to go through the code searching for switch statements who have an error case and have them throw RuntimeException for unreachable/unrecoverable cases. For example in util.MessageShape.getKindAnnotation.

@dwtj
Copy link
Collaborator

dwtj commented Sep 24, 2016

I agree that this is a missing check. Over on my fork, as part of commit 8ecb85c I've added a compile test to check for this case. The test results in an exception:

java.lang.IllegalArgumentException: Procedure labelled with @Duck is unduckable.

coming from org.paninij.proc.util.MessageShape:83.

I forget the details right now, but aren't there other cases where MessageShape determines that a procedure's shape is invalid. This makes me think that we don't just want a check that checks for when a void procedure has @duck on it; we want a check that will tell the user about any of the invalid cases detected by MessageShape.

@dwtj dwtj changed the title Missing Checks in Proc Missing check for @Duck void procedures Sep 24, 2016
@dwtj
Copy link
Collaborator

dwtj commented Sep 24, 2016

Note that I've renamed the title of this issue to focus its scope to just this one missing check.

@dwtj
Copy link
Collaborator

dwtj commented Sep 24, 2016

@jlmaddox, I agree that it would be good to identify switch cases with error cases and to loudly throw errors from them. Ought we to make a separate issue for that? It looks like you made a branch on your fork related to this. What's the status?

@jlmaddox
Copy link
Collaborator Author

In regards to the branch 144b on my fork, I have covered all the possibilities I can find. However, I noticed that some of the switch cases that were already covered throw IllegalArgumentException despite the function not taking an argument (error would be due to the argument passed to the constructor instead). I was thinking of either changing them to IllegalStateExceptions or RuntimeExceptions but I wasn't sure which would be the better fit.

@dwtj
Copy link
Collaborator

dwtj commented Sep 25, 2016

Yes, either of those exceptions would probably be a better fit. I haven't looked at the code, but is it possible/easy/sensible to just make the determination of an invalid argument at construction-time? That way we could throw the exception from the constructor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants