Skip to content

Commit

Permalink
terms with placeables are not allowed, see phetsims/joist#994
Browse files Browse the repository at this point in the history
  • Loading branch information
jessegreenberg committed Feb 5, 2025
1 parent ebc912c commit 14042c7
Showing 1 changed file with 27 additions and 4 deletions.
31 changes: 27 additions & 4 deletions js/browser-and-node/FluentLibrary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,23 @@ import { Visitor } from '../../../sherpa/lib/fluent/fluent-syntax-0.19.0/src/vis
* A visitor that collects Nodes from the AST so we can inspect them for problems.
*/
class FluentVisitor extends Visitor {
public readonly usedTerms = new Set<string>();
public readonly usedTermNames = new Set<string>();
public readonly usedTerms = new Set<Pattern>();
public readonly foundJunk = new Set<string>();
public readonly declaredTerms = new Set<string>();

public override visitTerm( node: IntentionalAny ): void {
this.declaredTerms.add( node );

this.genericVisit( node );
}

// IntentionalAny because the node type could not be found in Fluent source.
public override visitTermReference( node: IntentionalAny ): void {

// Add the term name to the set of used terms
this.usedTerms.add( node.id.name );
this.usedTermNames.add( node.id.name );
this.usedTerms.add( node );

// Continue traversing the AST
this.genericVisit( node );
Expand Down Expand Up @@ -67,6 +76,9 @@ class FluentLibrary {
* - Message keys should use camelCase instead of dashes.
* - All terms used in the file should be defined.
* - All selectors must have a default case.
* - Terms are not allowed to use placeables because terms used in placeables cannot take forwarded variables.
* - This is PhET specific and was added because it an easy programming mistake to assume that terms with
* placeables can be used like messages. This can be relaxed if needed in the future.
*/
public static verifyFluentFile( fluentFileString: string ): void {
const parser = new FluentParser();
Expand All @@ -85,12 +97,11 @@ class FluentLibrary {
.filter( entry => entry.type === 'Term' )
.map( entry => entry.id.name );

// Use the FluentVisitor to find all used terms.
const collector = new FluentVisitor();
collector.visit( resource );

// Identify used terms that are not defined
const undefinedTerms = Array.from( collector.usedTerms ).filter(
const undefinedTerms = Array.from( collector.usedTermNames ).filter(
term => !termKeys.includes( term )
);

Expand All @@ -99,6 +110,18 @@ class FluentLibrary {
throw new Error( `These terms are not defined: [ ${undefinedTermsFormatted} ]` );
}

// Terms are not allowed to use placeables because terms used in placeables cannot take forwarded variables.
// This is a PhET specific catch. Fluent allows translators to use terms with placeables and specify cases
// in the translation, see https://projectfluent.org/fluent/guide/terms.html. But it is an easy programming
// mistake to assume that terms with placeables can be used like messages so we catch it loudly here.
Array.from( collector.declaredTerms ).forEach( ( term: IntentionalAny ) => {
if ( term && term.value &&
term.value.elements &&
term.value.elements.some( ( element: IntentionalAny ) => element.type === 'Placeable' ) ) {
throw new Error( `Terms with placeables are not allowed: -${term.id.name} ` );
}
} );

// Other problems found by the collector.
collector.foundJunk.forEach( ( junk: IntentionalAny ) => {

Expand Down

0 comments on commit 14042c7

Please sign in to comment.