Skip to content

Add @MqlUnchecked #1068

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

Closed

Conversation

stIncMale
Copy link
Member

No description provided.

@stIncMale stIncMale mentioned this pull request Jan 18, 2023
@@ -108,7 +109,7 @@ public interface Expression {
IntegerExpression isIntegerOr(IntegerExpression other);
StringExpression isStringOr(StringExpression other);
DateExpression isDateOr(DateExpression other);
<T extends Expression> ArrayExpression<T> isArrayOr(ArrayExpression<? extends T> other);
<T extends Expression> ArrayExpression<@MqlUnchecked T> isArrayOr(ArrayExpression<? extends T> other);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this.

I think we should have no default, and specify the issue each time. For example, here, it should perhaps be MqlUnchecked(GENERIC)? This way, the user might click on the term to get guidance.

Copy link
Member Author

Choose a reason for hiding this comment

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

MqlUnchecked(TYPE_ARGUMENT) then I'd say, as the inferred/user-specified type argument of the parameterized type returned by the method is exactly where the API gives up on checking.

@@ -43,6 +45,7 @@ default BooleanExpression getBoolean(final String fieldName, final boolean other
return getBoolean(fieldName, of(other));
}

@MqlUnchecked(TYPE)
NumberExpression getNumber(String fieldName);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it should be called MqlAsserted(TYPE) instead? It seems to read a bit more easily, and suggest something on the user's part, rather than something perhaps unavoidable in the API? Unsure.

Copy link
Member Author

@stIncMale stIncMale Jan 18, 2023

Choose a reason for hiding this comment

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

While the Java SE API does not provide conceptually similar methods, the Java language has a concept of unchecked conversions. The Rust standard library provides functions where a user is supposed to guarantee something that is not checked by the function; it uses the "unchecked" suffix (sometimes prefix/infix) in the names of those functions, and even has an explicit corresponding naming convention.

That's why I think that "unchecked" is a better word - we are following existing conventions.

@stIncMale stIncMale requested a review from katcharov January 18, 2023 16:44
@stIncMale stIncMale changed the base branch from expressions to expressions-docs1 January 18, 2023 19:31
@stIncMale stIncMale marked this pull request as ready for review January 18, 2023 19:35
@stIncMale stIncMale force-pushed the expressions-unchecked branch 2 times, most recently from 2ea1ad6 to 0340e19 Compare January 18, 2023 23:33
* something that is not checked at run-time.
* If the assertion turns out to be false, the API behavior is unspecified.
*
* <p>This class is not part of the public API and may be removed or changed at any time</p>
Copy link
Member Author

@stIncMale stIncMale Jan 19, 2023

Choose a reason for hiding this comment

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

Note how this annotation is documented as not being a part of the public API despite residing in a non-internal package and being public. This is because we want to generate the API docs for this annotation, but we don't want users to use it either to annotate their code, or to analyze the code at compile time.

@katcharov katcharov force-pushed the expressions-unchecked branch from a1a9244 to 0bfce26 Compare January 19, 2023 16:10
@stIncMale stIncMale force-pushed the expressions-unchecked branch 2 times, most recently from 0d72b17 to 95efb31 Compare January 19, 2023 16:50
@stIncMale stIncMale force-pushed the expressions-unchecked branch from 95efb31 to cce2afc Compare January 20, 2023 00:29
@stIncMale stIncMale closed this Jan 20, 2023
@stIncMale stIncMale deleted the expressions-unchecked branch January 20, 2023 20:01
@stIncMale stIncMale self-assigned this Feb 19, 2023
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.

2 participants