Skip to content

Commit

Permalink
Policy check fix before minor release (#15636)
Browse files Browse the repository at this point in the history
## Description

Policy check fix before minor release
  • Loading branch information
jatgarg authored May 17, 2023
1 parent a182e25 commit deaf8b9
Show file tree
Hide file tree
Showing 15 changed files with 112 additions and 83 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ export function buildViewSchemaCollection(

for (const [key, field] of library.globalFieldSchema) {
// This check is an assert since if it fails, the other error messages would be incorrect.
assert(field.builder.name === library.name, "field must be part by the library its in");
assert(
field.builder.name === library.name,
0x6a8 /* field must be part by the library its in */,
);
const existing = globalFieldSchema.get(key);
if (existing !== undefined) {
errors.push(
Expand All @@ -59,7 +62,10 @@ export function buildViewSchemaCollection(
}
for (const [key, tree] of library.treeSchema) {
// This check is an assert since if it fails, the other error messages would be incorrect.
assert(tree.builder.name === library.name, "tree must be part by the library its in");
assert(
tree.builder.name === library.name,
0x6a9 /* tree must be part by the library its in */,
);
const existing = treeSchema.get(key);
if (existing !== undefined) {
errors.push(
Expand Down Expand Up @@ -114,7 +120,7 @@ export function validateViewSchemaCollection(collection: ViewSchemaCollection2):

// Validate that all schema referenced are included, and none are "never".
for (const [key, field] of collection.globalFieldSchema) {
assert(key === field.key, "field key should match map key");
assert(key === field.key, 0x6aa /* field key should match map key */);
validateGlobalField(collection, field, errors);
}
for (const [identifier, tree] of collection.treeSchema) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export class SchemaBuilder {
t: T,
): TreeSchema<Name, T> {
const schema = new TreeSchema(this, name, t);
assert(!this.treeSchema.has(schema.name), "Conflicting TreeSchema names");
assert(!this.treeSchema.has(schema.name), 0x6ab /* Conflicting TreeSchema names */);
this.treeSchema.set(schema.name, schema as TreeSchema);
return schema;
}
Expand Down Expand Up @@ -150,7 +150,7 @@ export class SchemaBuilder {
field: FieldSchema<Kind, Types>,
): GlobalFieldSchema<Kind, Types> {
const schema = new GlobalFieldSchema(this, brand(key), field);
assert(!this.globalFieldSchema.has(schema.key), "Conflicting global field keys");
assert(!this.globalFieldSchema.has(schema.key), 0x6ac /* Conflicting global field keys */);
this.globalFieldSchema.set(schema.key, schema);
return schema;
}
Expand Down Expand Up @@ -238,7 +238,7 @@ export class SchemaBuilder {
}

private finalize(): void {
assert(!this.finalized, "SchemaBuilder can only be finalized once.");
assert(!this.finalized, 0x6ad /* SchemaBuilder can only be finalized once. */);
this.finalized = true;
this.libraries.add({
name: this.name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ function normalizeField<T extends FieldSchema | undefined>(t: T): NormalizeField
return FieldSchema.empty as unknown as NormalizeField<T>;
}

assert(t instanceof FieldSchema, "invalid FieldSchema");
assert(t instanceof FieldSchema, 0x6ae /* invalid FieldSchema */);
return t as NormalizeField<T>;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,13 +288,13 @@ export function downCast<TSchema extends TreeSchema>(
const contextSchema = tree[contextSymbol].schema;
const lookedUp = contextSchema.treeSchema.get(schema.name);
// TODO: for this to pass, schematized view must have the view schema, not just stored schema.
assert(lookedUp === schema, "cannot downcast to a schema the tree is not using");
assert(lookedUp === schema, 0x68c /* cannot downcast to a schema the tree is not using */);

// TODO: make this actually work
const matches = tree[typeSymbol] === schema;
assert(
matches === (tree[typeSymbol].name === schema.name),
"schema object identity comparison should match identifier comparison",
0x68d /* schema object identity comparison should match identifier comparison */,
);
return matches;
}
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ function composeMarks<TNodeChange>(
} else if (!markHasCellEffect(newMark)) {
const moveInId = getMarkMoveId(baseMark);
if (nodeChange !== undefined && moveInId !== undefined) {
assert(isMoveMark(baseMark), "Only move marks have move IDs");
assert(isMoveMark(baseMark), 0x68e /* Only move marks have move IDs */);
getOrAddEffect(
moveEffects,
CrossFieldTarget.Source,
Expand All @@ -196,7 +196,10 @@ function composeMarks<TNodeChange>(
const moveOutId = getMarkMoveId(newMark);

if (moveInId !== undefined && moveOutId !== undefined) {
assert(isMoveMark(baseMark) && isMoveMark(newMark), "Only move marks have move IDs");
assert(
isMoveMark(baseMark) && isMoveMark(newMark),
0x68f /* Only move marks have move IDs */,
);
const srcEffect = getOrAddEffect(
moveEffects,
CrossFieldTarget.Source,
Expand Down Expand Up @@ -228,7 +231,7 @@ function composeMarks<TNodeChange>(
}

if (moveInId !== undefined) {
assert(isMoveMark(baseMark), "Only move marks have move IDs");
assert(isMoveMark(baseMark), 0x690 /* Only move marks have move IDs */);
getOrAddEffect(
moveEffects,
CrossFieldTarget.Source,
Expand All @@ -240,7 +243,7 @@ function composeMarks<TNodeChange>(
}

if (moveOutId !== undefined) {
assert(isMoveMark(newMark), "Only move marks have move IDs");
assert(isMoveMark(newMark), 0x691 /* Only move marks have move IDs */);

// The nodes attached by `baseMark` have been moved by `newMark`.
// We can represent net effect of the two marks by moving `baseMark` to the destination of `newMark`.
Expand Down Expand Up @@ -270,7 +273,7 @@ function createModifyMark<TNodeChange>(
return { count: cellId === undefined ? length : 0 };
}

assert(length === 1, "A mark with a node change must have length one");
assert(length === 1, 0x692 /* A mark with a node change must have length one */);
const mark: Modify<TNodeChange> = { type: "Modify", changes: nodeChange };
if (cellId !== undefined) {
mark.detachEvent = cellId;
Expand Down Expand Up @@ -463,10 +466,13 @@ export class ComposeQueue<T> {
} else if (areOutputCellsEmpty(baseMark) && areInputCellsEmpty(newMark)) {
// TODO: `baseMark` might be a MoveIn, which is not an ExistingCellMark.
// See test "[Move ABC, Return ABC] ↷ Delete B" in sequenceChangeRebaser.spec.ts
assert(isExistingCellMark(baseMark), "Only existing cell mark can have empty output");
assert(
isExistingCellMark(baseMark),
0x693 /* Only existing cell mark can have empty output */,
);
let baseCellId: DetachEvent;
if (markEmptiesCells(baseMark)) {
assert(isDetachMark(baseMark), "Only detach marks can empty cells");
assert(isDetachMark(baseMark), 0x694 /* Only detach marks can empty cells */);
const baseRevision = baseMark.revision ?? this.baseMarks.revision;
const baseIntention = getIntention(baseRevision, this.revisionMetadata);
if (baseRevision === undefined || baseIntention === undefined) {
Expand All @@ -477,7 +483,7 @@ export class ComposeQueue<T> {
// (which requires the local changes to have a revision tag))
assert(
isNewAttach(newMark),
"TODO: Assign revision tags to each change in a transaction",
0x695 /* TODO: Assign revision tags to each change in a transaction */,
);
return this.dequeueNew();
}
Expand All @@ -488,7 +494,7 @@ export class ComposeQueue<T> {
} else {
assert(
areInputCellsEmpty(baseMark),
"Mark with empty output must either be a detach or also have input empty",
0x696 /* Mark with empty output must either be a detach or also have input empty */,
);
baseCellId = baseMark.detachEvent;
}
Expand Down Expand Up @@ -584,7 +590,7 @@ export class ComposeQueue<T> {
const newMark = this.newMarks.peek();
assert(
baseMark !== undefined && newMark !== undefined,
"Cannot dequeue both unless both mark queues are non-empty",
0x697 /* Cannot dequeue both unless both mark queues are non-empty */,
);
const length = Math.min(getMarkLength(newMark), getMarkLength(baseMark));
return {
Expand All @@ -611,7 +617,7 @@ function areInverseMoves(
): boolean {
assert(
baseMark.type === "MoveIn" || baseMark.type === "ReturnTo",
"TODO: Handle case where `baseMark` is a detach",
0x698 /* TODO: Handle case where `baseMark` is a detach */,
);
if (baseMark.type === "ReturnTo" && baseMark.detachEvent?.revision === newIntention) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,10 @@ function applyMoveEffectsToDest<T>(
};

if (newMark.type === "ReturnTo" && newMark.detachEvent !== undefined) {
assert(effect.count !== undefined, "Should have a count when splitting a mark");
assert(
effect.count !== undefined,
0x699 /* Should have a count when splitting a mark */,
);
newMark.detachEvent = {
...newMark.detachEvent,
index: newMark.detachEvent.index + effect.count,
Expand Down Expand Up @@ -308,7 +311,10 @@ function applyMoveEffectsToSource<T>(
count: childEffect.count,
};
if (newMark.detachEvent !== undefined) {
assert(effect.count !== undefined, "Should specify a count when splitting a mark");
assert(
effect.count !== undefined,
0x69a /* Should specify a count when splitting a mark */,
);
newMark.detachEvent = {
...newMark.detachEvent,
index: newMark.detachEvent.index + effect.count,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export function rebase<TNodeChange>(
manager: CrossFieldManager,
revisionMetadata: RevisionMetadataSource,
): Changeset<TNodeChange> {
assert(base.revision !== undefined, "Cannot rebase over changeset with no revision");
assert(base.revision !== undefined, 0x69b /* Cannot rebase over changeset with no revision */);
const baseInfo =
base.revision === undefined ? undefined : revisionMetadata.getInfo(base.revision);
const baseIntention = baseInfo?.rollbackOf ?? base.revision;
Expand Down Expand Up @@ -311,7 +311,7 @@ class RebaseQueue<T> {
const newMark = this.newMarks.peek();
assert(
baseMark !== undefined && newMark !== undefined,
"Cannot dequeue both unless both mark queues are non-empty",
0x69c /* Cannot dequeue both unless both mark queues are non-empty */,
);
const length = Math.min(getMarkLength(newMark), getMarkLength(baseMark));
return {
Expand Down Expand Up @@ -359,7 +359,7 @@ function rebaseMark<TNodeChange>(

assert(
!isNewAttach(rebasedMark),
"A new attach should not be rebased over its cell being emptied",
0x69d /* A new attach should not be rebased over its cell being emptied */,
);

if (isMoveMark(rebasedMark)) {
Expand All @@ -383,7 +383,7 @@ function rebaseMark<TNodeChange>(
} else if (markFillsCells(baseMark)) {
assert(
isExistingCellMark(rebasedMark),
"Only an ExistingCellMark can target an empty cell",
0x69e /* Only an ExistingCellMark can target an empty cell */,
);
if (isMoveMark(rebasedMark)) {
if (rebasedMark.type === "MoveOut" || rebasedMark.type === "ReturnFrom") {
Expand Down Expand Up @@ -461,7 +461,7 @@ function makeDetachedMark<T>(
return { count: 0 };
}

assert(mark.detachEvent === undefined, "Expected mark to be attached");
assert(mark.detachEvent === undefined, 0x69f /* Expected mark to be attached */);
return { ...mark, detachEvent: { revision: detachIntention, index: offset } };
}

Expand Down Expand Up @@ -634,7 +634,7 @@ function compareCellPositions(
gapOffsetInBase: number,
): number {
const baseId = getCellId(baseMark, baseIntention);
assert(baseId !== undefined, "baseMark should have cell ID");
assert(baseId !== undefined, 0x6a0 /* baseMark should have cell ID */);
const newId = getCellId(newMark, undefined);
if (baseId.revision === newId?.revision) {
return baseId.index - newId.index;
Expand Down Expand Up @@ -675,7 +675,7 @@ function compareCellPositions(

assert(
isNewAttach(baseMark),
"Lineage should determine order of marks unless one is a new attach",
0x6a1 /* Lineage should determine order of marks unless one is a new attach */,
);

// `newMark` points to cells which were emptied before `baseMark` was created.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export function sequenceFieldToDelta<TNodeChange>(
} else {
// Inline into `switch(mark.type)` once we upgrade to TS 4.7
const type = mark.type;
assert(type !== NoopMarkType, "Cell changing mark must no be a NoopMark");
assert(type !== NoopMarkType, 0x6b0 /* Cell changing mark must no be a NoopMark */);
switch (type) {
case "Insert": {
const cursors = mark.content.map(singleTextCursor);
Expand Down Expand Up @@ -115,7 +115,7 @@ function deltaFromNodeChange<TNodeChange>(
if (change === undefined) {
return length;
}
assert(length === 1, "Modifying mark must be length one");
assert(length === 1, 0x6a3 /* Modifying mark must be length one */);
const modify = deltaFromChild(change);
return isEmptyModify(modify) ? 1 : modify;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class IndexTracker {
return;
}

assert(!isNoopMark(mark) && !isModify(mark), "These marks have no cell effects");
assert(!isNoopMark(mark) && !isModify(mark), 0x6a4 /* These marks have no cell effects */);

const netLength = outLength - inLength;
// If you hit this assert, then you probably need to add a check for it in `isNetZeroNodeCountChange`.
Expand Down Expand Up @@ -87,7 +87,10 @@ export class GapTracker {
if (!markHasCellEffect(mark)) {
this.map.clear();
} else {
assert(!isNoopMark(mark) && !isModify(mark), "These marks have no cell effects");
assert(
!isNoopMark(mark) && !isModify(mark),
0x6a5 /* These marks have no cell effects */,
);
const revision = mark.revision;
// TODO: Remove this early return. It is only needed because some tests use anonymous changes.
// These tests will fail (i.e., produce the wrong result) if they rely the index tracking performed here.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ export function tryExtendMark<T>(
}

if (isExistingCellMark(lhs)) {
assert(isExistingCellMark(rhs), "Should be existing cell mark");
assert(isExistingCellMark(rhs), 0x6a6 /* Should be existing cell mark */);
if (lhs.detachEvent?.revision !== rhs.detachEvent?.revision) {
return false;
}
Expand Down Expand Up @@ -1052,7 +1052,10 @@ export function withNodeChange<TNodeChange>(
return changes !== undefined ? { type: "Modify", changes } : mark;
case "MoveIn":
case "ReturnTo":
assert(changes === undefined, "Cannot have a node change on a MoveIn or ReturnTo mark");
assert(
changes === undefined,
0x6a7 /* Cannot have a node change on a MoveIn or ReturnTo mark */,
);
return mark;
case "Delete":
case "Insert":
Expand Down
10 changes: 5 additions & 5 deletions experimental/dds/tree2/src/shared-tree-core/branch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export class SharedTreeBranch<TEditor extends ChangeFamilyEditor, TChange> exten
*/
public setHead(head: GraphCommit<TChange>): void {
this.assertNotDisposed();
assert(!this.isTransacting(), "Cannot set head during a transaction");
assert(!this.isTransacting(), 0x685 /* Cannot set head during a transaction */);
this.head = head;
}

Expand Down Expand Up @@ -322,7 +322,7 @@ export class SharedTreeBranch<TEditor extends ChangeFamilyEditor, TChange> exten
public undo(): [change: TChange, newCommit: GraphCommit<TChange>] | undefined {
assert(
this.undoRedoManager !== undefined,
"Must construct branch with an `UndoRedoManager` in order to undo.",
0x686 /* Must construct branch with an `UndoRedoManager` in order to undo. */,
);
// TODO: allow this once it becomes possible to compose the changesets created by edits made
// within transactions and edits that represent completed transactions.
Expand All @@ -345,7 +345,7 @@ export class SharedTreeBranch<TEditor extends ChangeFamilyEditor, TChange> exten
public redo(): [change: TChange, newCommit: GraphCommit<TChange>] | undefined {
assert(
this.undoRedoManager !== undefined,
"Must construct branch with an `UndoRedoManager` in order to redo.",
0x687 /* Must construct branch with an `UndoRedoManager` in order to redo. */,
);
// TODO: allow this once it becomes possible to compose the changesets created by edits made
// within transactions and edits that represent completed transactions.
Expand Down Expand Up @@ -410,7 +410,7 @@ export class SharedTreeBranch<TEditor extends ChangeFamilyEditor, TChange> exten
// TODO: We probably can rebase a revertible branch onto a non-revertible branch.
assert(
branch.undoRedoManager !== undefined,
"Cannot rebase a revertible branch onto a non-revertible branch",
0x688 /* Cannot rebase a revertible branch onto a non-revertible branch */,
);
this.undoRedoManager.updateAfterRebase(sourceCommits, branch.undoRedoManager);
}
Expand Down Expand Up @@ -452,7 +452,7 @@ export class SharedTreeBranch<TEditor extends ChangeFamilyEditor, TChange> exten
// TODO: We probably can merge a non-revertible branch into a revertible branch.
assert(
branch.undoRedoManager !== undefined,
"Cannot merge a non-revertible branch into a revertible branch",
0x689 /* Cannot merge a non-revertible branch into a revertible branch */,
);
this.undoRedoManager.updateAfterMerge(sourceCommits, branch.undoRedoManager);
}
Expand Down
2 changes: 1 addition & 1 deletion experimental/dds/tree2/src/shared-tree-core/editManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ export class EditManager<
public loadSummaryData(data: SummaryData<TChangeset>): void {
assert(
this.isEmpty(),
"Attempted to load from summary after edit manager was already mutated",
0x68a /* Attempted to load from summary after edit manager was already mutated */,
);
this.sequenceMap.clear();
this.trunk.setHead(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ export class SharedTreeCore<TEditor extends ChangeFamilyEditor, TChange> extends
// Edits should not be submitted until all transactions finish
assert(
!this.getLocalBranch().isTransacting(),
"Unexpected edit submitted during transaction",
0x68b /* Unexpected edit submitted during transaction */,
);

// Edits submitted before the first attach are treated as sequenced because they will be included
Expand Down
Loading

0 comments on commit deaf8b9

Please sign in to comment.