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

Java-like annotations in comments #503

Merged
merged 19 commits into from
Jan 29, 2021
Merged

Conversation

konnov
Copy link
Collaborator

@konnov konnov commented Jan 28, 2021

This PR addresses two related issues: #262 and #226. Instead of developing a special solution for type annotations, we produce a simple and general syntax of annotations, like in Java. To make use of the annotations, this PR also extends SanyImporter to parse annotations in the comments that are written in front of declarations. The annotation syntax and examples are given in ADR004 (this PR).

On top of that, TestAnnotationParser is the first property-based test in Apalache :-)

  • Tests added for any new code
  • Documentation added for any new functionality
  • Entry added to UNRELEASED.md for any new functionality

@konnov
Copy link
Collaborator Author

konnov commented Jan 28, 2021

@shonfeder the formatting check fails on a markdown file. What shall we do?

Copy link
Contributor

@shonfeder shonfeder left a comment

Choose a reason for hiding this comment

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

I left some comments, but I'm hoping the size of this PR will be very reduced once the formatting is fixed (as per the failing CI job) so I'll hold of on reviewing more until then.

UNRELEASED.md Show resolved Hide resolved
docs/src/adr/004adr-annotations.md Outdated Show resolved Hide resolved
docs/src/adr/004adr-annotations.md Outdated Show resolved Hide resolved
docs/src/adr/004adr-annotations.md Outdated Show resolved Hide resolved
A more concise syntax for type annotations could be as follows:

```tla
\* @type: Int => Int
Copy link
Contributor

Choose a reason for hiding this comment

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

Would we have the same problem with @type(Int => Int)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, the types may contain parentheses, that will confuse the annotation parser a lot. These are two separate parsers: one of java-like annotations, and one for types.

Copy link
Collaborator Author

@konnov konnov Jan 29, 2021

Choose a reason for hiding this comment

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

The problem is that we don't have an end marker, which is usually \n. How about adding the infamous semicolon in the end of a quote-free annotation:

\* @type: (Int, Int) => Int ;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have added this special form to the annotations. It definitely has less clutter when writing type annotations.

docs/src/adr/004adr-annotations.md Outdated Show resolved Hide resolved
test/tla/Annotations.tla Show resolved Hide resolved
@shonfeder
Copy link
Contributor

@konnov did you already try make fmt-fix?

@konnov
Copy link
Collaborator Author

konnov commented Jan 28, 2021

interesting. I forgot to do it. Can we add it to a PR template.

@konnov
Copy link
Collaborator Author

konnov commented Jan 28, 2021

There are plenty of small replacements in files, basically adding one more parameter annotationStore: AnnotationStore.

Copy link
Contributor

@shonfeder shonfeder left a comment

Choose a reason for hiding this comment

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

LGTM

@konnov
Copy link
Collaborator Author

konnov commented Jan 29, 2021

I have resolved the conflicts. The last commit is massive, as it merges the formatting reversal from unstable

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