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

Emit events for all known types #127

Merged
merged 46 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
fdc909b
Emit events about all types
dmdashenkov Mar 16, 2023
a4f32df
Add a test for non-request files
dmdashenkov Mar 16, 2023
1534077
Bump version
dmdashenkov Mar 17, 2023
77a711d
Add doc
dmdashenkov Mar 17, 2023
c551eb9
Add definitions for a dependency-based view
dmdashenkov Mar 17, 2023
643ad58
Implement a view collecting dependency file data
dmdashenkov Mar 17, 2023
b5963fd
Move some implementation details to `:compiler`
dmdashenkov Mar 20, 2023
f8ca578
Emit events for dependencies
dmdashenkov Mar 22, 2023
9811ba2
More testing
dmdashenkov Mar 22, 2023
a412d59
Add file path in services and remove the `shouldGenerate` flag
dmdashenkov Mar 22, 2023
ea7025a
Split new model-building code into a separate file
dmdashenkov Mar 22, 2023
47884bb
Assemble separate functions into a context-class
dmdashenkov Mar 24, 2023
b363bf2
Add missing type details
dmdashenkov Mar 24, 2023
7a0617e
Add a test for ensuring event-based and procedure-based model builder…
dmdashenkov Mar 24, 2023
85c211e
Clean up Proto definition
dmdashenkov Mar 24, 2023
b105840
Fix imports
dmdashenkov Mar 24, 2023
e078f86
More doc
dmdashenkov Mar 24, 2023
281d9b1
Revert changes from `config`
dmdashenkov Mar 27, 2023
2527f70
More details on the new compiler event
dmdashenkov Mar 27, 2023
040fb73
Better naming
dmdashenkov Mar 27, 2023
ee687ae
Style fixes
dmdashenkov Mar 27, 2023
d6d2708
Make a destructing call more obvious
dmdashenkov Mar 27, 2023
f4c7a76
Fix a typo
dmdashenkov Mar 27, 2023
83f51c7
Improve naming and add static imports
dmdashenkov Mar 27, 2023
5de89bc
More static imports
dmdashenkov Mar 27, 2023
514ce41
Yet more static imports
dmdashenkov Mar 27, 2023
e88a6bd
Yet more static imports
dmdashenkov Mar 27, 2023
342ecc2
Use Kotlin DSL for Proto messages where possible
dmdashenkov Mar 27, 2023
37ad5f3
Revert `config` changes
dmdashenkov Mar 27, 2023
1a25dbd
Kotlinize message building
dmdashenkov Mar 28, 2023
5ebf770
Use Kotest asasertions
dmdashenkov Mar 28, 2023
6dddbbe
Better naming for the new view
dmdashenkov Mar 28, 2023
796127d
Extract common functionality
dmdashenkov Mar 28, 2023
51ed614
Simplify proto definition construction
dmdashenkov Mar 28, 2023
07dea0f
Improve readability
dmdashenkov Mar 28, 2023
c477f1b
Fix type nullability
dmdashenkov Mar 30, 2023
c8b2a60
Refactor functions to avoid `this@smth`-s
dmdashenkov Mar 30, 2023
0fa23c7
Extract duplicates
dmdashenkov Mar 30, 2023
d6133ec
Repackage event factories
dmdashenkov Mar 30, 2023
af59295
Fix typo
dmdashenkov Mar 30, 2023
cd18964
Add doc
dmdashenkov Mar 30, 2023
113e558
Fix typo
dmdashenkov Mar 30, 2023
518c8b2
Clear up imports and use Kotest where possible
dmdashenkov Mar 30, 2023
8e207c1
Remove redundant `Companion` specifier
dmdashenkov Mar 30, 2023
5a11ddd
Add static imports
dmdashenkov Mar 30, 2023
ff3ab1e
Use `val` instead of `fun`
dmdashenkov Mar 30, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build-on-ubuntu.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
run: ./gradlew build --stacktrace

# See: https://github.com/marketplace/actions/junit-report-action
- name: Publish Unit Test Report
- name: Publish Test Report
uses: mikepenz/[email protected]
if: always() # always run even if the previous step fails
with:
Expand Down
6 changes: 3 additions & 3 deletions api/src/main/kotlin/io/spine/protodata/AstExtensions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -195,12 +195,12 @@ private val FileDescriptor.typeUrlPrefix: String
/**
* Obtains the name of this `oneof` as a [OneofName].
*/
internal fun OneofDescriptor.name(): OneofName = oneofName { value = name }
public fun OneofDescriptor.name(): OneofName = oneofName { value = name }

/**
* Obtains the name of this field as a [FieldName].
*/
internal fun FieldDescriptor.name(): FieldName = fieldName { value = name }
public fun FieldDescriptor.name(): FieldName = fieldName { value = name }

/**
* Obtains the relative path to this file as a [FilePath].
Expand All @@ -219,7 +219,7 @@ public fun ServiceDescriptor.name(): ServiceName = serviceName {
/**
* Obtains the name of this RPC method as an [RpcName].
*/
internal fun MethodDescriptor.name(): RpcName = rpcName { value = name }
public fun MethodDescriptor.name(): RpcName = rpcName { value = name }

/**
* Obtains a [Type] wrapping this `PrimitiveType`.
Expand Down
2 changes: 1 addition & 1 deletion api/src/main/kotlin/io/spine/protodata/plugin/Plugin.kt
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public interface Plugin {
*/
public fun BoundedContextBuilder.apply(plugin: Plugin) {
val repos = plugin.viewRepositories().toMutableList()
val defaultRepos = plugin.views().map { io.spine.protodata.plugin.ViewRepository.default(it) }
val defaultRepos = plugin.views().map { ViewRepository.default(it) }
repos.addAll(defaultRepos)
val repeatedView = repos.map { it.entityClass() }
.groupingBy { it }
Expand Down
3 changes: 3 additions & 0 deletions api/src/main/proto/spine/protodata/ast.proto
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,9 @@ message Service {

// Documentation of this service.
Doc doc = 4;

// Path to the file which declares this service.
FilePath file = 5;
}

// A name of an RPC method.
Expand Down
21 changes: 21 additions & 0 deletions api/src/main/proto/spine/protodata/events.proto
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ option java_outer_classname = "EventsProto";
option java_multiple_files = true;

import "spine/protodata/ast.proto";
import "spine/protodata/source.proto";

// Emitted when processing of a file begins.
message FileEntered {
Expand Down Expand Up @@ -264,3 +265,23 @@ message RpcExited {

RpcName rpc = 3;
}

// Emitted when the Protobuf compiler discovers a dependency file.
//
// The order of files reported by events of this type is unspecified. Users should not rely on any
// particular order.
//
// Each file is only reported by one event, even if the file is imported into
//
// `DependencyDiscovered` events always precede all other Protobuf compiler events, i.e.
// the first `FileEntered` will always be emitted after the last `DependencyDiscovered`.
//
// Normally, no code should be generated for dependencies. However, they can be used for additional
// info when generating code for the module's own types.
//
message DependencyDiscovered {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would be more precise with this name, so that Java- and Proto-level dependencies could be distinguished.

Copy link
Contributor Author

@dmdashenkov dmdashenkov Mar 27, 2023

Choose a reason for hiding this comment

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

I've updated the doc somewhat. I'd like to avoid adding the Proto prefix because:

  • we only work with Protobuf, no Java or JS or other dependency is ever considered in ProtoData (not talking about the Gradle compiler here, only ProtoData proper);
  • this is one of a series of events such as TypeEntered, OptionDiscovered, etc., which do not specify the language either.


FilePath file = 1;

ProtobufSourceFile content = 2;
}
15 changes: 14 additions & 1 deletion api/src/main/proto/spine/protodata/source.proto
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import "spine/protodata/ast.proto";
// Includes all the message types declared in the file, along with their fields.
//
message ProtobufSourceFile {
option (entity).kind = PROJECTION;
option (entity).kind = VIEW;

FilePath file_path = 1;

Expand Down Expand Up @@ -70,3 +70,16 @@ message ProtobufSourceFile {
//
map<string, Service> service = 5;
}

// A Protobuf definitions file which is included into the module as a dependency.
//
// Unlike `ProtobufSourceFile`, `ProtobufDependencyFile` should not result in any code generation.
// Rather, it serves an informative purpose.
//
message ProtobufDependencyFile {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the suffix File in this type and in ProtobufSourceFile makes it a bit harder to understand the difference. More, we have the file property under ProtobufSourceFile and content under ProtobufDependencyFile, which makes it a bit more confusing. In fact, the dependency is on the file. We just don't generate the code after it.

option (entity).kind = VIEW;

FilePath file_path = 1;

ProtobufSourceFile content = 2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we have it named as file instead, please?

}
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,5 @@ package io.spine.internal.dependency

// https://pmd.github.io/
object Pmd {
const val version = "6.51.0"
const val version = "6.55.0"
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class Spine(p: ExtensionAware) {
*
* @see [ProtoData]
*/
const val protoData = "0.7.4"
const val protoData = "0.7.6"

/**
* The default version of `base` to use.
Expand All @@ -85,7 +85,7 @@ class Spine(p: ExtensionAware) {
/**
* The version of `mc-java` to use.
*/
const val mcJava = "2.0.0-SNAPSHOT.134"
const val mcJava = "2.0.0-SNAPSHOT.132"

/**
* The version of `base-types` to use.
Expand Down Expand Up @@ -116,7 +116,7 @@ class Spine(p: ExtensionAware) {
* The version of `tool-base` to use.
* @see [Spine.toolBase]
*/
const val toolBase = "2.0.0-SNAPSHOT.161"
const val toolBase = "2.0.0-SNAPSHOT.156"

/**
* The version of `validation` to use.
Expand Down
198 changes: 198 additions & 0 deletions compiler/src/main/kotlin/io/spine/protodata/backend/AstComponents.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
/*
* Copyright 2023, TeamDev. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Redistribution and use in source and/or binary forms, with or without
* modification, must retain the above copyright notice and the following
* disclaimer.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

package io.spine.protodata.backend

import com.google.protobuf.Descriptors
import com.google.protobuf.Empty
import io.spine.protodata.EnumConstant
import io.spine.protodata.Field
import io.spine.protodata.Rpc
import io.spine.protodata.ServiceName
import io.spine.protodata.TypeName
import io.spine.protodata.cardinality
import io.spine.protodata.constantName
import io.spine.protodata.file
import io.spine.protodata.name
import io.spine.protodata.path

/**
* Converts this field descriptor into a [Field] with options.
*
* @see buildField
*/
internal fun Descriptors.FieldDescriptor.buildFieldWithOptions(
declaringType: TypeName,
documentation: Documentation
): Field {
val field = buildField(declaringType, documentation)
return field.toBuilder()
.addAllOption(listOptions(options))
.build()
}

/**
* Converts this field descriptor into a [Field].
*
* The resulting [Field] will not reflect the field options.
*
* @see buildFieldWithOptions
*/
internal fun Descriptors.FieldDescriptor.buildField(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please import Descriptors.FieldDescriptor and other types nested under Descriptors so that we have shorter declarations.

declaringType: TypeName,
documentation: Documentation
): Field {
return Field.newBuilder()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use Proto DSL here, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In methods that already have a receiver, I found it easier to use the builder rather than the DSL. Otherwise, we'd have to specify which this we're talking about each time:

this@field.number = this@buildField.number

It's similar in other places in AstComponents.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We do have name clashes in some cases, but they are rare. I've changed several such blocks myself and know what you're talking about.

Try to apply the DSL to this particular case and you'll see that the code becomes much more compact and easier to read.

.setName(name())
.setDeclaringType(declaringType)
.setNumber(number)
.setOrderOfDeclaration(index)
.assignTypeAndCardinality(this)
.setDoc(documentation.forField(this))
.build()
}

/**
* Assigns the field type and cardinality (`map`/`list`/`oneof_name`/`single`) to the receiver
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this, in fact, copying the field type and the cardinality? If so, maybe we could get rid of this "assign" term which feels a bit over-used in our domain?

* builder.
*
* @return the receiver for method chaining.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think, in Kotlin we still don't put periods in @return statement descriptions.

*/
private fun Field.Builder.assignTypeAndCardinality(
desc: Descriptors.FieldDescriptor
): Field.Builder {
if (desc.isMapField) {
val (keyField, valueField) = desc.messageType.fields
map = Field.OfMap.newBuilder()
.setKeyType(keyField.primitiveType())
.build()
type = valueField.type()
} else {
type = desc.type()
when {
desc.isRepeated -> list = Empty.getDefaultInstance()
desc.realContainingOneof != null -> oneofName = desc.realContainingOneof.name()
else -> single = Empty.getDefaultInstance()
}
}
return this
}

/**
* Converts this enum value descriptor into an [EnumConstant] with options.
*
* @see buildConstant
*/
internal fun Descriptors.EnumValueDescriptor.buildConstantWithOptions(
declaringType: TypeName,
documentation: Documentation
): EnumConstant {
val constant = buildConstant(declaringType, documentation)
return constant.toBuilder()
.addAllOption(listOptions(options))
.build()
}

/**
* Converts this enum value descriptor into an [EnumConstant].
*
* The resulting [EnumConstant] will not reflect the options on the enum constant.
*
* @see buildConstantWithOptions
*/
internal fun Descriptors.EnumValueDescriptor.buildConstant(
declaringType: TypeName,
documentation: Documentation
): EnumConstant {
return EnumConstant.newBuilder()
.setName(constantName { value = name })
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use Kotlin proto DSL here.

.setDeclaredIn(declaringType)
.setNumber(number)
.setOrderOfDeclaration(index)
.setDoc(documentation.forEnumConstant(this))
.build()
}

/**
* Converts this method descriptor into an [Rpc] with options.
*
* @see buildRpc
*/
internal fun Descriptors.MethodDescriptor.buildRpcWithOptions(
declaringService: ServiceName,
documentation: Documentation
) : Rpc {
return buildRpc(declaringService, documentation)
.toBuilder()
.addAllOption(listOptions(options))
.build()
}

/**
* Converts this method descriptor into an [Rpc].
*
* The resulting [Rpc] will not reflect the method options.
*
* @see buildRpcWithOptions
*/
internal fun Descriptors.MethodDescriptor.buildRpc(
declaringService: ServiceName,
documentation: Documentation
) : Rpc {
val name = name()
val cardinality = cardinality()
return Rpc.newBuilder()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use Kotlin proto DSL here.

.setName(name)
.setCardinality(cardinality)
.setRequestType(inputType.name())
.setResponseType(outputType.name())
.setDoc(documentation.forRpc(this))
.setService(declaringService)
.build()
}

/**
* Extracts metadata from this file descriptor, including file options.
*
* @see toFile
*/
internal fun Descriptors.FileDescriptor.toFileWithOptions() =
toFile()
.toBuilder()
.addAllOption(listOptions(options))
.build()

/**
* Extracts metadata from this file descriptor, excluding file options.
*
* @see toFileWithOptions
*/
internal fun Descriptors.FileDescriptor.toFile() = file {
path = path()
packageName = `package`
syntax = [email protected]()
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure whether we can remove this empty line (and not break EOF), but if we can, let's.

Loading