Merge pull request #19 from disneystreaming/dfrancoeur/proto-index

Tweak protoIndex validation to check union member

.github/workflows/ci.yml

@@ -30,29 +30,21 @@ jobs:
             distribution: adopt
             java-version: 11
 
-      - name: Get mill version
-        run: echo "MILL_VERSION=$(cat .mill-version)" >> $GITHUB_ENV
-
-      - name: Setup Mill
-        uses: jodersky/setup-mill@master
-        with:
-          mill-version: ${{ env.MILL_VERSION }}
-
       - name: Check formatting
-        run: mill -k --disable-ticker __.checkFormat
+        run: ./mill -k --disable-ticker __.checkFormat
 
       - name: Check headers
-        run: mill -k --disable-ticker __.headerCheck
+        run: ./mill -k --disable-ticker __.headerCheck
 
       - name: Compile
-        run: mill -k --disable-ticker __.compile
+        run: ./mill -k --disable-ticker __.compile
 
       - name: Run tests
-        run: mill -k --disable-ticker __.test
+        run: ./mill -k --disable-ticker __.test
 
       - name: Publish ${{ github.ref }}
         if: startsWith(github.ref, 'refs/tags/v')
-        run: mill -i io.kipp.mill.ci.release.ReleaseModule/publishAll
+        run: ./mill -i io.kipp.mill.ci.release.ReleaseModule/publishAll
         env:
           PGP_PASSPHRASE: ${{ secrets.PGP_PASSPHRASE }}
           PGP_SECRET: ${{ secrets.PGP_SECRET }}

.mill-version

@@ -1 +1 @@
-0.10.7
+0.10.9

README.md

@@ -344,6 +344,8 @@ message Test {
 }
 ```
 
+When one field is annotated with a `@protoIndex`, all fields have to be annotated with it. This includes the fields of any structure used within the structure.
+
 #### alloy.proto#protoNumType
 
 Specifies the type of signing that should be used for integers and longs. Options are:
@@ -370,11 +372,11 @@ For full documentation on what each of these traits does, see the smithy specifi
 ### Publish Local
 
 ```console
-> mill __.publishLocal
+> ./mill __.publishLocal
 ```
 
 ### Run Tests
 
 ```console
-> mill __.test
+> ./mill __.test
 ```

build.sc

@@ -1,4 +1,4 @@
-import $ivy.`de.tototec::de.tobiasroeser.mill.vcs.version::0.2.0`
+import $ivy.`de.tototec::de.tobiasroeser.mill.vcs.version::0.3.0`
 import de.tobiasroeser.mill.vcs.version.VcsVersion
 import $ivy.`io.github.davidgregory084::mill-tpolecat::0.3.0`
 import $ivy.`com.lewisjkl::header-mill-plugin::0.0.1`
@@ -80,23 +80,20 @@ trait BasePublishModule extends BaseModule with CiReleaseModule {
 trait BaseJavaModule extends JavaModule with BasePublishModule
 
 trait BaseScalaNoPublishModule
-    extends mill.scalalib.bsp.ScalaMetalsSupport
+    extends ScalaModule
     with ScalafmtModule
     with TpolecatModule {
   def scalaVersion = T.input("2.13.8")
-  def semanticDbVersion = T.input("4.4.34")
 }
 
 trait BaseScalaModule extends BaseScalaNoPublishModule with BasePublishModule
 
 trait BaseCrossScalaModule
-    extends mill.scalalib.bsp.ScalaMetalsSupport
+    extends ScalaModule
     with ScalafmtModule
     with TpolecatModule
     with CrossScalaModule
-    with BasePublishModule {
-  def semanticDbVersion = T.input("4.4.34")
-}
+    with BasePublishModule
 
 object core extends BaseJavaModule {
   def ivyDeps = Agg(

mill

@@ -0,0 +1,55 @@
+#!/usr/bin/env sh
+
+# This is a wrapper script, that automatically download mill from GitHub release pages
+# You can give the required mill version with MILL_VERSION env variable
+# If no version is given, it falls back to the value of DEFAULT_MILL_VERSION
+DEFAULT_MILL_VERSION=0.10.9
+
+set -e
+
+if [ -z "$MILL_VERSION" ] ; then
+  if [ -f ".mill-version" ] ; then
+    MILL_VERSION="$(head -n 1 .mill-version 2> /dev/null)"
+  elif [ -f ".config/mill-version" ] ; then
+    MILL_VERSION="$(head -n 1 .config/mill-version 2> /dev/null)"
+  elif [ -f "mill" ] && [ "$0" != "mill" ] ; then
+    MILL_VERSION=$(grep -F "DEFAULT_MILL_VERSION=" "mill" | head -n 1 | cut -d= -f2)
+  else
+    MILL_VERSION=$DEFAULT_MILL_VERSION
+  fi
+fi
+
+if [ "x${XDG_CACHE_HOME}" != "x" ] ; then
+  MILL_DOWNLOAD_PATH="${XDG_CACHE_HOME}/mill/download"
+else
+  MILL_DOWNLOAD_PATH="${HOME}/.cache/mill/download"
+fi
+MILL_EXEC_PATH="${MILL_DOWNLOAD_PATH}/${MILL_VERSION}"
+
+version_remainder="$MILL_VERSION"
+MILL_MAJOR_VERSION="${version_remainder%%.*}"; version_remainder="${version_remainder#*.}"
+MILL_MINOR_VERSION="${version_remainder%%.*}"; version_remainder="${version_remainder#*.}"
+
+if [ ! -s "$MILL_EXEC_PATH" ] ; then
+  mkdir -p "$MILL_DOWNLOAD_PATH"
+  if [ "$MILL_MAJOR_VERSION" -gt 0 ] || [ "$MILL_MINOR_VERSION" -ge 5 ] ; then
+    ASSEMBLY="-assembly"
+  fi
+  DOWNLOAD_FILE=$MILL_EXEC_PATH-tmp-download
+  MILL_VERSION_TAG=$(echo $MILL_VERSION | sed -E 's/([^-]+)(-M[0-9]+)?(-.*)?/\1\2/')
+  MILL_DOWNLOAD_URL="https://github.com/lihaoyi/mill/releases/download/${MILL_VERSION_TAG}/$MILL_VERSION${ASSEMBLY}"
+  curl --fail -L -o "$DOWNLOAD_FILE" "$MILL_DOWNLOAD_URL"
+  chmod +x "$DOWNLOAD_FILE"
+  mv "$DOWNLOAD_FILE" "$MILL_EXEC_PATH"
+  unset DOWNLOAD_FILE
+  unset MILL_DOWNLOAD_URL
+fi
+
+if [ -z "$MILL_MAIN_CLI" ] ; then
+  MILL_MAIN_CLI="${0}"
+fi
+
+unset MILL_DOWNLOAD_PATH
+unset MILL_VERSION
+
+exec $MILL_EXEC_PATH -D "mill.main.cli=${MILL_MAIN_CLI}" "$@"

modules/core/src/alloy/proto/validation/ProtoIndexTraitValidator.java

@@ -15,9 +15,7 @@
 
 package alloy.proto.validation;
 
-import java.util.Collection;
 import java.util.Collections;
-import java.util.function.Function;
 import java.util.List;
 import java.util.Optional;
 import java.util.Map;
@@ -43,14 +41,13 @@ public List<ValidationEvent> validate(Model model) {
 		final Set<ShapeId> uniqueShapes = model.getMemberShapesWithTrait(ProtoIndexTrait.class).stream()
 				.map(MemberShape::getContainer).collect(Collectors.toSet());
 		return uniqueShapes.stream().flatMap(shapeId -> OptionHelper.toStream(model.getShape(shapeId)))
-				.flatMap(c -> validateShape(c).stream()).collect(Collectors.toList());
+				.flatMap(c -> validateShape(model, c).stream()).collect(Collectors.toList());
 	}
 
-	private List<ValidationEvent> validateShape(Shape shape) {
-		final Map<String, MemberShape> members = allMembers(shape);
-		final Map<String, Optional<ProtoIndexTrait>> fieldsAndIndexes = members.entrySet().stream().collect(
-				Collectors.<Map.Entry<String, MemberShape>, String, Optional<ProtoIndexTrait>>toMap(e -> e.getKey(),
-						e -> e.getValue().getTrait(ProtoIndexTrait.class)));
+	private List<ValidationEvent> validateShape(Model model, Shape shape) {
+		final Map<String, Shape> members = allMembers(model, shape);
+		final Map<String, Optional<ProtoIndexTrait>> fieldsAndIndexes = members.entrySet().stream()
+				.collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().getTrait(ProtoIndexTrait.class)));
 		boolean allFieldsAreIndexed = fieldsAndIndexes.values().stream().allMatch(Optional::isPresent);
 		boolean noFieldIsIndexed = fieldsAndIndexes.values().stream().allMatch(o -> !o.isPresent());
 		boolean onlySomeFieldsAreIndexed = !(allFieldsAreIndexed || noFieldIsIndexed);
@@ -63,7 +60,7 @@ private List<ValidationEvent> validateShape(Shape shape) {
 			final Map<Integer, List<String>> perIndex = fieldsAndIndexes.entrySet().stream().flatMap(
 					e -> e.getValue().map(trait -> Stream.of(mapEntry(e.getKey(), trait))).orElse(Stream.empty()))
 					.collect(Collectors.groupingBy(e -> e.getValue().getNumber(),
-							Collectors.mapping(e -> e.getKey(), Collectors.toList())));
+							Collectors.mapping(Map.Entry::getKey, Collectors.toList())));
 
 			return perIndex.entrySet().stream().filter(e -> e.getValue().size() > 1).map(e -> {
 				final Integer index = e.getKey();
@@ -79,9 +76,34 @@ private List<ValidationEvent> validateShape(Shape shape) {
 		}
 	}
 
-	private Map<String, MemberShape> allMembers(Shape shape) {
-		return shape.asUnionShape().map(u -> u.getAllMembers())
-				.orElse(shape.asStructureShape().map(u -> u.getAllMembers()).orElse(Collections.emptyMap()));
+	/**
+	 * Simple helper to turn deal with java.util.Map not being covariant.
+	 */
+	private <T extends Shape> Map.Entry<String, Shape> asShape(Map.Entry<String, T> subShape) {
+		return mapEntry(subShape.getKey(), subShape.getValue());
+	}
+
+	private Optional<UnionShape> memberIsUnion(Model model, MemberShape shape) {
+		return model.getShape(shape.getTarget()).flatMap(Shape::asUnionShape);
+	}
+
+	private Map<String, Shape> allMembers(Model model, Shape shape) {
+		final Stream<Map.Entry<String, Shape>> unionMembers = OptionHelper.toStream(shape.asUnionShape())
+				.flatMap(u -> u.getAllMembers().entrySet().stream().map(this::asShape));
+
+		final Stream<Map.Entry<String, Shape>> structureMembers = OptionHelper.toStream(shape.asStructureShape())
+				.flatMap(u -> {
+					Stream<Map.Entry<String, Shape>> rootShapes = u.getAllMembers().entrySet().stream()
+							.filter(e -> !memberIsUnion(model, e.getValue()).isPresent()).map(this::asShape);
+					Stream<Map.Entry<String, Shape>> subUnionMembers = u.getAllMembers().values().stream()
+							.flatMap(m -> OptionHelper.toStream(memberIsUnion(model, m)))
+							.flatMap(union -> allMembers(model, union).entrySet().stream()
+								.map(e -> asShape(mapEntry(union.getId().getName() + "#" + e.getKey(), e.getValue())))
+							);
+					return Stream.concat(rootShapes, subUnionMembers);
+				});
+		return Stream.concat(unionMembers, structureMembers)
+				.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
 	}
 
 	private <K, V> Map.Entry<K, V> mapEntry(K key, V value) {