fix: stop after a silent delete or reject, and keep spaced rule names single

"Stop processing further rules" wrote no stop when the rule's last action
was "Delete silently" or "Reject". Both only cancel the implicit keep
(RFC 5228 4.4, RFC 5429): the script goes on, so a later rule still
filed the deleted or rejected message into a folder. Such a rule now
always writes a stop, unless its block has one already; rules below it
no longer run on its messages, which is what the option says.

Since 1.11.1 the "# Rule:" line carries the rule name with its runs of
whitespace collapsed, but the parser compared it with the name as
stored. A name like "Foo  Bar", one with a tab or a trailing space, or
an empty one came back as someone else's block on every read, and every
save added one more copy. Both sides are now compared collapsed, and the
name is read from its own line only. That also drops the copies earlier
saves left, as long as the rule still exists under that name.

lib/sieve/__tests__/generator.test.ts

@@ -241,22 +241,53 @@ describe('generateScript', () => {
       expect(matches).toHaveLength(1);
     });
 
-    it('does not append stop after discard', () => {
+    // discard and reject only cancel the implicit keep (RFC 5228 4.4,
+    // RFC 5429): without a stop, the rules below still act on the message.
+    it('appends stop after discard', () => {
       const script = generateScript([makeRule({
         actions: [{ type: 'discard' }],
         stopProcessing: true,
       })]);
-      const matches = script.match(/stop;/g);
-      expect(matches).toBeNull();
+      expect(script).toContain('    discard;\n    stop;\n}');
+      expect(script.match(/stop;/g)).toHaveLength(1);
     });
 
-    it('does not append stop after reject', () => {
+    it('appends stop after reject', () => {
       const script = generateScript([makeRule({
         actions: [{ type: 'reject', value: 'No' }],
         stopProcessing: true,
       })]);
-      const matches = script.match(/stop;/g);
-      expect(matches).toBeNull();
+      expect(script).toContain('    reject "No";\n    stop;\n}');
+      expect(script.match(/stop;/g)).toHaveLength(1);
+    });
+
+    it('leaves a rule that does not stop processing open after discard', () => {
+      const script = generateScript([makeRule({
+        actions: [{ type: 'discard' }],
+        stopProcessing: false,
+      })]);
+      expect(script).not.toContain('stop;');
+    });
+
+    it('writes one stop when the stop action ends up last behind the flags', () => {
+      // Flags are written first, so the stop action placed before a flag in
+      // the list still comes last.
+      const script = generateScript([makeRule({
+        actions: [{ type: 'stop' }, { type: 'mark_read' }],
+        stopProcessing: true,
+      })]);
+      expect(script).toContain('    addflag "\\\\Seen";\n    stop;\n}');
+      expect(script.match(/stop;/g)).toHaveLength(1);
+    });
+
+    it('adds no second stop behind a stop action earlier in the block', () => {
+      // The block runs straight through, so that stop ends the script.
+      const script = generateScript([makeRule({
+        actions: [{ type: 'stop' }, { type: 'discard' }],
+        stopProcessing: true,
+      })]);
+      expect(script).toContain('    stop;\n    discard;\n}');
+      expect(script.match(/stop;/g)).toHaveLength(1);
     });
   });
 

lib/sieve/__tests__/parser.test.ts

@@ -122,6 +122,53 @@ describe('parseScript', () => {
       expect(result.rules).toEqual(rules);
     });
 
+    it('reads a rule whose name has runs of whitespace back as one rule, save after save', () => {
+      // A hand-written block, and one that only looks like Bulwark's.
+      const external = '# Hand\nif header :contains "subject" "hand" {\n    keep;\n}\n'
+        + '# Rule: Someone else\nif header :contains "subject" "other" {\n    keep;\n}\n';
+      const rules = [
+        makeRule({ id: '1', name: 'Foo  Bar' }),
+        makeRule({ id: '2', name: ' Tab\tand  trailing ' }),
+        // No-break and ideographic spaces count as whitespace too.
+        makeRule({ id: '3', name: 'Büro 　Post' }),
+        // Not possible in the editor, but in metadata written elsewhere.
+        makeRule({ id: '4', name: '' }),
+      ];
+      let script = `${generateScript(rules)}\n${external}`;
+      for (let save = 0; save < 3; save++) {
+        const result = parseScript(script);
+        expect(result.isOpaque).toBe(false);
+        // The four rules, and each other block once.
+        const own = result.rules.filter((r) => (r.origin ?? 'bulwark') === 'bulwark');
+        expect(own.map((r) => r.id)).toEqual(['1', '2', '3', '4']);
+        expect(result.rules.length - own.length).toBe(2);
+        script = generateScript(result.rules, result.vacation, { externalRequires: result.externalRequires });
+      }
+      expect(script.split('# Rule: Foo Bar\n').length).toBe(2);
+      expect(script.split('# Rule: Büro Post\n').length).toBe(2);
+      expect(script.split('# Rule: \n').length).toBe(2);
+      expect(script.split('if header :contains "subject" "hand"').length).toBe(2);
+      expect(script.split('# Rule: Someone else').length).toBe(2);
+    });
+
+    it('takes a block written before names were collapsed for the rule itself', () => {
+      // Up to 1.11.0 the "# Rule:" line carried the name as typed.
+      const rule = makeRule({ id: '1', name: 'Foo  Bar' });
+      const asWritten = generateScript([rule]).replace('# Rule: Foo Bar', '# Rule: Foo  Bar');
+      expect(asWritten).toContain('# Rule: Foo  Bar');
+      expect(parseScript(asWritten).rules).toEqual([rule]);
+    });
+
+    it('drops the copies of such a rule an earlier save left behind', () => {
+      const rule = makeRule({ id: '1', name: 'Foo  Bar' });
+      const block = generateScript([rule]).slice(generateScript([rule]).indexOf('# Rule: Foo Bar'));
+      // As written before: the rule, and two copies read back as someone else's.
+      const withCopies = `${generateScript([rule])}\n# --- External rules (managed outside Bulwark) ---\n${block}\n${block}`;
+      const result = parseScript(withCopies);
+      expect(result.rules).toEqual([rule]);
+      expect(generateScript(result.rules).split('# Rule: Foo Bar').length).toBe(2);
+    });
+
     it('preserves rules with special characters', () => {
       const rules = [makeRule({
         conditions: [{ field: 'subject', comparator: 'contains', value: 'say "hello" \\ world' }],

lib/sieve/generator.ts

@@ -359,11 +359,13 @@ export function generateScript(
 
     const actionLines = generateActions(rule.actions, useMailboxId);
 
-    if (rule.stopProcessing) {
-      const lastAction = rule.actions[rule.actions.length - 1];
-      if (!lastAction || !['stop', 'discard', 'reject'].includes(lastAction.type)) {
-        actionLines.push('stop;');
-      }
+    // discard and reject only cancel the implicit keep (RFC 5228 4.4,
+    // RFC 5429); the script goes on, and the rules below would still act on
+    // the message. So "stop processing" always writes a stop, unless the
+    // block has one already: it runs straight through, so any stop in it ends
+    // the script.
+    if (rule.stopProcessing && !actionLines.includes('stop;')) {
+      actionLines.push('stop;');
     }
 
     lines.push(`if ${conditionStr} {`);

lib/sieve/parser.ts

@@ -847,10 +847,15 @@ export function parseScript(content: string): ParseResult {
     // identifies it as ours.
     const filteredExternal = external.rules.filter(r => {
       const raw = r.rawBlock || '';
-      const match = raw.match(/#\s*Rule:\s*(.+?)\s*$/m);
+      // The name ends with its line, and may be empty.
+      const match = raw.match(/#\s*Rule:[ \t]*(.*?)[ \t]*$/m);
       if (match) {
-        const name = match[1].trim();
-        if (bulwarkRules.some(b => b.name === name)) return false;
+        // The generator writes the name on one line, its whitespace runs
+        // collapsed; compared as written, "Foo  Bar" would come back as
+        // someone else's rule, and as one more copy on every save.
+        const oneLine = (name: string) => name.replace(/\s+/g, ' ').trim();
+        const name = oneLine(match[1]);
+        if (bulwarkRules.some(b => oneLine(b.name) === name)) return false;
       }
       if (/#\s*Vacation auto-reply/i.test(raw)) return false;
       return true;