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

Add Annotation Inheritance, tests, and docs #654

Merged
merged 8 commits into from
Sep 4, 2023

Conversation

briandowns
Copy link
Contributor

@briandowns briandowns commented Sep 3, 2023

Resolves: #633

What's Changed:

Added the ability to have classes inherit class, method, and field annotations.

Type of Change:

  • Bug fix
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Housekeeping:

  • Tests have been updated to reflect the changes done within this PR (if applicable).
  • Documentation has been updated to reflect the changes done within this PR (if applicable).

Screenshots (If Applicable):

@briandowns briandowns changed the title add annotation inheritance, tests, and docs Add Annotation Inheritance, tests, and docs Sep 3, 2023
@briandowns briandowns changed the title Add Annotation Inheritance, tests, and docs [WIP] - Add Annotation Inheritance, tests, and docs Sep 3, 2023
@briandowns briandowns changed the title [WIP] - Add Annotation Inheritance, tests, and docs Add Annotation Inheritance, tests, and docs Sep 3, 2023
Signed-off-by: Brian Downs <brian.downs@gmail.com>
Signed-off-by: Brian Downs <brian.downs@gmail.com>
src/vm/vm.c Outdated Show resolved Hide resolved
src/vm/vm.c Outdated Show resolved Hide resolved
Signed-off-by: Brian Downs <brian.downs@gmail.com>
@Jason2605
Copy link
Member

Couple of things we will need to think about with this when testing actually - the first is if the child class overrides something in the parent class we want to take the childs implementation not the parents

class Base {
    @method
    test() {

    }
}

class Test < Base {
    @method1234
    test() {

    }
}

print(Test.methodAnnotations); // {"test": {"method": nil}}

// Where I would expect {"test": {"method1234": nil}}

I would imagine some sort of key check in copyAnnotations should do the trick.

Similarly (not sure why this would happen) but if we modify the annotation value on the child we want to make sure it doesn't update the parent (essentially we would want a deep copy)

class Base {
    @method
    test() {

    }
}

class Test < Base {

}

Test.methodAnnotations['test']['method'] = 10;

print(Test.methodAnnotations); // {"test": {"method": 10}}
print(Base.methodAnnotations); // {"test": {"method": 10}}

Signed-off-by: Brian Downs <brian.downs@gmail.com>
Signed-off-by: Brian Downs <brian.downs@gmail.com>
…ld is updated

Signed-off-by: Brian Downs <brian.downs@gmail.com>
Signed-off-by: Brian Downs <brian.downs@gmail.com>
@briandowns
Copy link
Contributor Author

Great catches. Impl updated as well as tests.

Copy link
Member

@Jason2605 Jason2605 left a comment

Choose a reason for hiding this comment

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

Sorry couple more comments!

src/vm/vm.c Outdated Show resolved Hide resolved
src/vm/vm.c Outdated Show resolved Hide resolved
src/vm/vm.c Outdated Show resolved Hide resolved
src/vm/vm.c Outdated Show resolved Hide resolved
Signed-off-by: Brian Downs <brian.downs@gmail.com>
@briandowns
Copy link
Contributor Author

All set!

@Jason2605
Copy link
Member

Nicceeee thanks for this!!

@Jason2605 Jason2605 merged commit 6d68acd into dictu-lang:develop Sep 4, 2023
8 checks passed
@briandowns briandowns deleted the isssue-633 branch September 4, 2023 23:53
@briandowns
Copy link
Contributor Author

No prob. Thanks for the quick reviews! My ORM is really coming along now that all of the annotation features are in!

@Jason2605
Copy link
Member

Awesome! Definitely be interested to see it! Also side note, I ended up using Wanbli for a little side project of mine :)

@briandowns
Copy link
Contributor Author

Open to any and all feedback on that. I don't really have it in a stable state yet so use with caution!

@Jason2605
Copy link
Member

It’s all good just a little tool I’m using in the house to track some smart plug usage! I’ll potentially PR some things across for it but it’s been a fun project

@briandowns
Copy link
Contributor Author

Here's the ORM. Suuuuper naive... Not finished but works. https://github.com/briandowns/kahless . Gonna get an example together using Kahless and Wanbli together.

@Jason2605
Copy link
Member

This looks really cool!

This was referenced Oct 31, 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.

[FEATURE] - Classes should inherit annotations from inherited classes
2 participants