Fix issues related to payees declared on posting's metadata

Payees declared on posting's metadata are now validated with `--check-payees` option. Also, their aliases are now considered on reports as well.
ledger · Sep 22, 2020 · fc8e4ce · fc8e4ce
1 parent c796746
commit fc8e4ce
Show file tree

Hide file tree

Showing 8 changed files with 87 additions and 30 deletions.
diff --git a/doc/ledger3.texi b/doc/ledger3.texi
@@ -3104,10 +3104,6 @@ date is not repeated), but they have different payees now.
 If using the @option{--strict} or @option{--pedantic} options, you must
 declare this tag to avoid warnings and errors.
 
-The payee name used with the tag is not enforced by the
-@option{--check-payees} option, due to a bug:
-@url{https://github.com/ledger/ledger/issues/556}.
-
 @node Metadata values, Typed metadata, Metadata tags, Metadata
 @subsection Metadata values
 

diff --git a/src/journal.cc b/src/journal.cc
@@ -213,27 +213,43 @@ account_t * journal_t::expand_aliases(string name) {
   return result;
 }
 
-string journal_t::register_payee(const string& name, xact_t * xact)
+string journal_t::register_payee(const string& name)
 {
-  string payee;
+  if (should_check_payees() && payee_not_registered(name)) {
+    known_payees.insert(name);
+  }
 
-  if (check_payees &&
-      (checking_style == CHECK_WARNING || checking_style == CHECK_ERROR)) {
-    std::set<string>::iterator i = known_payees.find(name);
+  return name;
+}
 
-    if (i == known_payees.end()) {
-      if (! xact) {
-        known_payees.insert(name);
-      }
-      else if (checking_style == CHECK_WARNING) {
-        current_context->warning(_f("Unknown payee '%1%'") % name);
-      }
-      else if (checking_style == CHECK_ERROR) {
-        throw_(parse_error, _f("Unknown payee '%1%'") % name);
-      }
+string journal_t::validate_payee(const string& name_or_alias)
+{
+  string payee = translate_payee_name(name_or_alias);
+
+  if (should_check_payees() && payee_not_registered(payee)) {
+    if (checking_style == CHECK_WARNING) {
+      current_context->warning(_f("Unknown payee '%1%'") % payee);
+    }
+    else if (checking_style == CHECK_ERROR) {
+      throw_(parse_error, _f("Unknown payee '%1%'") % payee);
     }
   }
 
+  return payee;
+}
+
+bool journal_t::should_check_payees() {
+  return check_payees &&
+      (checking_style == CHECK_WARNING || checking_style == CHECK_ERROR);
+}
+
+bool journal_t::payee_not_registered(const string& name) {
+  return known_payees.find(name) == known_payees.end();
+}
+
+string journal_t::translate_payee_name(const string& name) {
+  string payee;
+
   foreach (payee_alias_mapping_t& value, payee_alias_mappings) {
     if (value.first.match(name)) {
       payee = value.second;

diff --git a/src/journal.h b/src/journal.h
@@ -157,7 +157,8 @@ class journal_t : public noncopyable
 
   account_t * register_account(const string& name, post_t * post,
                                account_t * master = NULL);
-  string      register_payee(const string& name, xact_t * xact);
+  string      register_payee(const string& name);
+  string      validate_payee(const string& name_or_alias);
   void        register_commodity(commodity_t& comm,
                                  variant<int, xact_t *, post_t *> context);
   void        register_metadata(const string& key, const value_t& value,
@@ -194,7 +195,12 @@ class journal_t : public noncopyable
   bool valid() const;
 
 private:
+
   std::size_t read_textual(parse_context_stack_t& context);
+
+  bool        should_check_payees();
+  bool        payee_not_registered(const string& name);
+  string      translate_payee_name(const string& name);
 };
 
 } // namespace ledger

diff --git a/src/post.cc b/src/post.cc
@@ -120,12 +120,22 @@ optional<date_t> post_t::aux_date() const
   return date;
 }
 
-string post_t::payee() const
+string post_t::payee_from_tag() const
 {
   if (optional<value_t> post_payee = get_tag(_("Payee")))
     return post_payee->as_string();
   else
-    return xact->payee;
+    return "";
+}
+
+string post_t::payee() const
+{
+  if (_payee)
+    return *_payee;
+
+  string post_payee = payee_from_tag();
+
+  return post_payee != "" ? post_payee : xact->payee;
 }
 
 namespace {

diff --git a/src/post.h b/src/post.h
@@ -73,6 +73,12 @@ class post_t : public item_t
   optional<datetime_t> checkin;
   optional<datetime_t> checkout;
 
+private:
+
+  optional<string> _payee;
+
+public:
+
   post_t(account_t * _account = NULL,
          flags_t     _flags   = ITEM_NORMAL)
     : item_t(_flags), xact(NULL), account(_account)
@@ -132,7 +138,11 @@ class post_t : public item_t
   virtual date_t primary_date() const;
   virtual optional<date_t> aux_date() const;
 
+  string payee_from_tag() const;
   string payee() const;
+  void set_payee(const string& payee) {
+    _payee = payee;
+  }
 
   bool must_balance() const {
     return ! has_flags(POST_VIRTUAL) || has_flags(POST_MUST_BALANCE);

diff --git a/src/textual.cc b/src/textual.cc
@@ -1033,7 +1033,7 @@ void instance_t::account_value_directive(account_t * account, string expr_str)
 
 void instance_t::payee_directive(char * line)
 {
-  string payee = context.journal->register_payee(line, NULL);
+  string payee = context.journal->register_payee(line);
 
   while (peek_whitespace_line()) {
     read_line(line);
@@ -1752,6 +1752,10 @@ post_t * instance_t::parse_post(char *          line,
   foreach (string& tag, tags)
     post->parse_tags(tag.c_str(), *context.scope, true);
 
+  string post_payee = post->payee_from_tag();
+  if (post_payee != "")
+    post->set_payee(context.journal->validate_payee(post_payee));
+
   TRACE_STOP(post_details, 1);
 
   return post.release();
@@ -1870,7 +1874,7 @@ xact_t * instance_t::parse_xact(char *          line,
       }
       ++p;
     }
-    xact->payee = context.journal->register_payee(next, xact.get());
+    xact->payee = context.journal->validate_payee(next);
     next = p;
   } else {
     xact->payee = _("<Unspecified payee>");

diff --git a/test/baseline/dir-payee.test b/test/baseline/dir-payee.test
@@ -8,6 +8,10 @@ payee Foo Bar Inc
     A            10
     B
 
+2012-03-26 * KFC
+    A            10 ; Payee: Kentucky Fried Chicken
+    B
+
 2014-05-13 * UNHELPFUL PAYEE  ; will be read as being 'Foo Bar Inc'
     ; UUID: 2a2e21d434356f886c84371eebac6e44f1337fda
     A            20
@@ -16,6 +20,8 @@ payee Foo Bar Inc
 test reg
 12-Mar-25 KFC                   A                                10           10
                                 B                               -10            0
+12-Mar-26 KFC                   A                                10           10
+                                B                               -10            0
 14-May-13 Foo Bar Inc           A                                20           20
                                 B                               -20            0
 end test

diff --git a/test/baseline/opt-check-payees.test b/test/baseline/opt-check-payees.test
@@ -5,7 +5,10 @@ account Expenses:Food
 commodity EUR
 commodity GBP
 payee Phone
+    alias MobilePhone
+payee Several
 tag food
+tag Payee
 
 2012-03-20 Phone
     Expenses:Phone            20.00 GBP
@@ -20,18 +23,24 @@ tag food
     Expenses:Food             20.00 EUR
     Assets:Cash
 
+2012-03-23 Several
+    Expenses:Food              10.00 EUR ; Payee: Food
+    Expenses:Phone             10.00 EUR ; Payee: MobilePhone
+    Assets:Cash
+
 test bal --strict --check-payees
-          -20.00 EUR
+          -40.00 EUR
          -570.00 GBP  Assets:Cash
-           20.00 EUR
+           40.00 EUR
           570.00 GBP  Expenses
-           20.00 EUR    Food
+           30.00 EUR    Food
+           10.00 EUR
            20.00 GBP    Phone
           550.00 GBP    Rent
 --------------------
                    0
 __ERROR__
-Warning: "$FILE", line 14: Unknown payee 'Rent'
-Warning: "$FILE", line 18: Unknown payee 'Food'
+Warning: "$FILE", line 17: Unknown payee 'Rent'
+Warning: "$FILE", line 21: Unknown payee 'Food'
+Warning: "$FILE", line 27: Unknown payee 'Food'
 end test
-