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

Implement @ThrowsException annotation #135

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

d367wang
Copy link

@d367wang d367wang commented Jun 3, 2020

This PR implements @ThrowsException annotation and fixes issue #2076

@d367wang d367wang requested a review from wmdietl June 3, 2020 20:50
@wmdietl wmdietl self-assigned this Jun 15, 2020
@wmdietl
Copy link
Member

wmdietl commented Jun 22, 2020

@lnsun Can you have a pass through these changes?

@wmdietl wmdietl assigned lnsun and unassigned wmdietl Jun 22, 2020
@lnsun
Copy link
Member

lnsun commented Jun 22, 2020

@lnsun Can you have a pass through these changes?

@wmdietl Sure!

@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
public @interface ThrowsException {
Class<? extends Throwable> value() default Throwable.class;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to make this an array, since a method could throw multiple different exceptions.

@@ -300,6 +303,13 @@ public static ControlFlowGraph build(
/** Does this node terminate the execution? (e.g., "System.exit()") */
protected boolean terminatesExecution = false;

/**
* Provided this node terminates the execution, does the execution exit immediately? e.g.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about "Indicate whether this node terminates the execution of current method and exits the program immediately"

AnnotationUtils.getElementValueClassName(throwAnno, "value", false)
.toString();
if (cls == null) { // thrown type is Throwable by default
thrown = elements.getTypeElement("java.lang.Throwable").asType();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default is already decleared in the annotation as Throwable, is this line accessible in any way?

// get type of thrown exception
String cls =
AnnotationUtils.getElementValueClassName(throwAnno, "value", false)
.toString();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If value is changing to array, maybe this part needs to be updated

@lnsun
Copy link
Member

lnsun commented Jun 22, 2020

@wmdietl @d367wang I just finished the review! Feel free to reach out if there's anything unclear or wrong!

P.S. This change seems a useful one. Aren't we pushing it to typetools?

@lnsun lnsun assigned wmdietl and unassigned lnsun Jun 22, 2020
@xingweitian xingweitian assigned d367wang and unassigned wmdietl Jun 28, 2020
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
public @interface ThrowsException {
Class<? extends RuntimeException> value() default RuntimeException.class;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About the choice of the type of value, I think there are the following questions:

  1. Whether Error should be included.
  2. Should the checked exceptions be included? Although uncaught checked exceptions are always declared in the throws clause in the method signature, it cannot express the behavior of "unconditionally throws such exception", which can be supplemented by @ThrowsException annotation.

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

Successfully merging this pull request may close these issues.

3 participants