Skip to content

Commit

Permalink
Merge pull request #17 from publicodes/fix-raw-rules-parsing
Browse files Browse the repository at this point in the history
fix: don't overwrite namespaces when importing rules in existing ones
  • Loading branch information
Clemog authored Jul 8, 2024
2 parents e2575b1 + 9f2199d commit ef3d6f2
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 28 deletions.
3 changes: 2 additions & 1 deletion client/src/test/diagnostics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import { getDocUri, activate, mainUri, mainPath } from "./helper";
* 4. For each test:
* a. copy the content of the test document to the main document,
* b. save the main document and wait for the diagnostics to be computed.
*
* TODO: Add at least one test for each diagnostic (need to support multiple files).
*/

suite("Should get diagnostics", () => {
test("Missing name in import macros", async () => {
await testDiagnostics(getDocUri("diagnostics-import.publicodes"), [
Expand Down
66 changes: 42 additions & 24 deletions server/src/parseRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from "./context";
import { getTSTree } from "./treeSitter";
import { mapAppend, positionToRange, trimQuotedString } from "./helpers";
import { RuleName } from "@publicodes/tools";

export const PUBLICODES_FILE_EXTENSIONS = [
".publicodes",
Expand Down Expand Up @@ -61,29 +62,38 @@ export function parseDocument(
const fileInfos = ctx.fileInfos.get(filePath);
const tsTree = getTSTree(fileContent, fileInfos, document);
const { rawRules, errors } = parseRawRules(filePath);
const { definitions, importNamespace } = collectRuleDefs(tsTree.rootNode);

const ruleDefs = collectRuleDefs(tsTree.rootNode).filter(
({ dottedName, namesPos }) => {
const ruleFilePath = ctx.ruleToFileNameMap.get(dottedName);
const ruleDefs = definitions.filter(({ dottedName, namesPos }) => {
const ruleFilePath = ctx.ruleToFileNameMap.get(dottedName);

// Check if the rule is already defined in another file
if (ruleFilePath && ruleFilePath !== filePath) {
errors.push({
severity: DiagnosticSeverity.Error,
range: positionToRange(namesPos),
message: `[ Erreur syntaxique ]
La règle '${dottedName}' est déjà définie dans le fichier : "${ruleFilePath}".
// Check if the rule is already defined in another file
// TODO: add a test case for this
if (ruleFilePath && ruleFilePath !== filePath) {
errors.push({
severity: DiagnosticSeverity.Error,
range: positionToRange(namesPos),
message: `[ Erreur syntaxique ]
La règle '${dottedName}' est déjà définie dans le fichier : '${ruleFilePath}'.
[ Solutions ]
- Renommez une des définitions de la règle '${dottedName}'.
- Supprimez une des définitions de la règle '${dottedName}'.`,
});
delete rawRules[dottedName];
return false;
}
return true;
},
);
});
delete rawRules[dottedName];
return false;
}
return true;
});

// Checks if the namespace is not already defined in another file
// TODO: add a warning if the namespace is already defined in another file
if (importNamespace) {
const ruleFilePath = ctx.ruleToFileNameMap.get(importNamespace);
if (ruleFilePath && ruleFilePath !== filePath) {
delete rawRules[importNamespace];
}
}

ctx.fileInfos.set(filePath, {
// NOTE: not needed for now (we use the parsedRules from the engine)
Expand Down Expand Up @@ -111,6 +121,10 @@ function parseRawRules(filePath: FilePath): {
} {
const errors: Diagnostic[] = [];
try {
// FIXME: for now, we only call getModelFromSource to resolve imports
// and map potential errors to the current file. We should have a
// better error handling mechanism in the future to only call
// getModelFromSource once in validate.ts.
const resolvedRules = getModelFromSource(filePath);
return { rawRules: resolvedRules, errors };
} catch (e: any) {
Expand Down Expand Up @@ -222,24 +236,27 @@ L'attribut '${name}' doit être suivi d'une valeur.
function collectRuleDefs(
node: TSParser.SyntaxNode,
parentRule?: DottedName,
): RuleDef[] {
const rules: RuleDef[] = [];
): { definitions: RuleDef[]; importNamespace: RuleName | undefined } {
const definitions: RuleDef[] = [];
// Namespace where the rules are imported (either the package name or the
// content of the `dans` node).
let importNamespace: RuleName | undefined;

node.children.forEach((child) => {
if (child.type === "import") {
const packageName = resolvePackageName(child);
importNamespace = resolvePackageName(child);

child.childForFieldName("rules")?.namedChildren.forEach((rule) => {
if (rule.type === "rule" || rule.type === "import_rule") {
rules.push(...getRuleDefsInRule(rule, packageName));
definitions.push(...getRuleDefsInRule(rule, importNamespace));
}
});
} else if (child.type === "rule") {
rules.push(...getRuleDefsInRule(child, parentRule));
definitions.push(...getRuleDefsInRule(child, parentRule));
}
});

return rules;
return { definitions, importNamespace };
}

function getRuleDefsInRule(
Expand Down Expand Up @@ -281,7 +298,8 @@ function getRuleDefsInRule(
if (bodyNode && bodyNode.type === "rule_body") {
bodyNode.namedChildren.forEach((child) => {
if (child.type === "s_avec") {
rules.push(...collectRuleDefs(child, dottedName));
const { definitions } = collectRuleDefs(child, dottedName);
rules.push(...definitions);
}
});
}
Expand Down
16 changes: 15 additions & 1 deletion server/src/semanticTokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,6 @@ function collectTokens(
case "plafond":
case "arrondi":
case "montant":
case "dans":
case "références_à":
case "sauf_dans":
case "choix_obligatoire":
Expand Down Expand Up @@ -233,6 +232,21 @@ function collectTokens(
break;
}

case "import": {
const intoNode = node.childForFieldName("into");
if (intoNode && intoNode.type === "dotted_name") {
intoNode.children.forEach((name) => {
pushToken(
builder,
name.startPosition.row,
name.startPosition.column,
name.endPosition.column - name.startPosition.column,
SemanticTokenTypes.namespace,
);
});
}
}

case "import_rule":
case "reference": {
const dottedName = node.firstNamedChild;
Expand Down
5 changes: 3 additions & 2 deletions server/src/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ import { fileURLToPath, pathToFileURL } from "node:url";
import { Logger } from "publicodes";
import { mapAppend, positionToRange } from "./helpers";
import { getRefInRule } from "./treeSitter";
import { getModelFromSource } from "@publicodes/tools/compilation";

export default async function validate(
ctx: LSContext,
document?: TextDocument,
): Promise<void> {
ctx.diagnostics = new Map();
let startTimer = Date.now();

if (document) {
const docFilePath = fileURLToPath(document.uri);
Expand All @@ -30,7 +32,6 @@ export default async function validate(
};
});

let startTimer = Date.now();
ctx.engine = new Engine(ctx.rawPublicodesRules, {
logger: getDiagnosticsLogger(ctx),
});
Expand Down Expand Up @@ -65,7 +66,7 @@ export default async function validate(
}

ctx.connection.console.log(
`[validate] Found ${ctx.diagnostics.size} diagnostics.`,
`[validate] Found ${ctx.diagnostics.size} diagnostics in ${Date.now() - startTimer}ms.`,
);
sendDiagnostics(ctx);
}
Expand Down

0 comments on commit ef3d6f2

Please sign in to comment.