diff --git a/src/server/verify.js b/src/server/verify.js index 1911a86..a4c8db9 100644 --- a/src/server/verify.js +++ b/src/server/verify.js @@ -4,7 +4,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { lintSubmission } from '../shared/lint.js'; -import { freeTextErrors } from '../shared/freetext.js'; +import { freeTextErrorsAcross } from '../shared/freetext.js'; import { checkUnits } from '../shared/check.js'; import { reconcile } from '../shared/reconcile.js'; import { STATUS, sameOriginal } from '../shared/compare.js'; @@ -225,16 +225,14 @@ export class Verifier { /** * Comments and strings a submission adds that could carry instructions to other agents: only * annotations and markers, and strings of the original executable, are allowed (freetext.js). + * What the submitted files held on the base may move between them (a definition moving to its + * owner keeps its provenance), but every such line counts once. */ freeTextGate(files, baseCommit) { const exe = this.getPe().buf; const inOriginal = (bytes) => exe.indexOf(bytes) !== -1; - const errors = []; - for (const [file, text] of Object.entries(files)) { - if (typeof text !== 'string') continue; - for (const e of freeTextErrors(text, this.repo.show(baseCommit, file), inOriginal)) errors.push({ file, ...e }); - } - return errors; + const pooled = Object.keys(files).map((f) => this.repo.show(baseCommit, f)).filter((t) => t !== null).join('\n'); + return freeTextErrorsAcross(files, pooled, inOriginal); } /** diff --git a/src/shared/freetext.js b/src/shared/freetext.js index 5360895..b77a1ad 100644 --- a/src/shared/freetext.js +++ b/src/shared/freetext.js @@ -160,18 +160,35 @@ function encode(s, wide) { * @returns {{line: number, message: string}[]} the messages never repeat the text itself */ export function freeTextErrors(text, baseText, inOriginal) { - const now = scanText(text); - const base = baseText === null ? { comments: [], strings: [] } : scanText(baseText); - const errors = []; + return checkText(scanText(text), baseText === null ? { comments: [], strings: [] } : scanText(baseText), inOriginal); +} - const baseComments = counts(base.comments.map((c) => c.text.trim())); +/** + * The same check for a whole submission against what all its files held together on the base: + * comments and strings may move from one submitted file to another, each base occurrence once. + * @returns {{file: string, line: number, message: string}[]} + */ +export function freeTextErrorsAcross(files, pooledBase, inOriginal) { + const base = scanText(pooledBase); + const pool = { comments: counts(base.comments.map((c) => c.text.trim())), strings: counts(base.strings.map(stringKey)) }; + const errors = []; + for (const [file, text] of Object.entries(files)) { + if (typeof text !== 'string') continue; + for (const e of checkText(scanText(text), pool, inOriginal)) errors.push({ file, ...e }); + } + return errors; +} + +function checkText(now, base, inOriginal) { + const errors = []; + const baseComments = base.comments instanceof Map ? base.comments : counts(base.comments.map((c) => c.text.trim())); for (const c of now.comments) { const t = c.text.trim(); if (take(baseComments, t) || allowedComment(t)) continue; errors.push({ line: c.line, message: 'comments are not allowed in submissions (only annotations like // FUNCTION: fn_00001 and offset markers like /* 0010 */); remove it' }); } - const baseStrings = counts(base.strings.map(stringKey)); + const baseStrings = base.strings instanceof Map ? base.strings : counts(base.strings.map(stringKey)); for (const s of now.strings) { if (take(baseStrings, stringKey(s))) continue; if (s.include !== null) { diff --git a/test/freetext.test.js b/test/freetext.test.js index ead67d9..5dbacb9 100644 --- a/test/freetext.test.js +++ b/test/freetext.test.js @@ -1,6 +1,6 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { scanText, freeTextErrors, allowedComment } from '../src/shared/freetext.js'; +import { scanText, freeTextErrors, freeTextErrorsAcross, allowedComment } from '../src/shared/freetext.js'; // Stands in for the original executable's bytes. const original = Buffer.concat([Buffer.from('Interface\\GlueXML\0', 'latin1'), Buffer.from('hello world\0%d\0', 'latin1'), Buffer.from('Wide\0', 'utf16le')]); @@ -51,6 +51,14 @@ test('comments and strings already on main stay allowed', () => { assert.equal(freeTextErrors(base + '// keep me\n', base, inOriginal).length, 1); }); +test('a submission may move a comment from one of its files to another, once', () => { + const base = '// keep me\nextern int g;\n'; + const files = { 'a.cpp': 'extern int g;\n', 'b.cpp': '// keep me\nint g;\n' }; + assert.deepEqual(freeTextErrorsAcross(files, base, inOriginal), [], 'moved with its definition'); + const twice = { 'a.cpp': base, 'b.cpp': '// keep me\nint g;\n' }; + assert.equal(freeTextErrorsAcross(twice, base, inOriginal).length, 1, 'one base line covers one copy'); +}); + test('pieces of a concatenated literal may each come from the original', () => { assert.deepEqual(freeTextErrors('const char* s = "%d" "hello world";\n', null, inOriginal), []); assert.equal(freeTextErrors('const char* s = "%d" "made up";\n', null, inOriginal).length, 1);