pyln: Turn the plugin options into a real Type (uppercase T)

[ Fix not to include 'value' and 'default' (if None) in getmanifest response --RR ]
[ Fix to support [] operator for existing plugins (including our test ones!) --RR ]

contrib/pyln-client/pyln/client/plugin.py

@@ -1,10 +1,3 @@
-from .lightning import LightningRpc, Millisatoshi
-from binascii import hexlify
-from collections import OrderedDict
-from enum import Enum
-from threading import RLock
-from typing import Any, Callable, Dict, List, Optional, Tuple, Union
-
 import inspect
 import io
 import json
@@ -14,6 +7,14 @@ import os
 import re
 import sys
 import traceback
+from binascii import hexlify
+from collections import OrderedDict
+from dataclasses import dataclass
+from enum import Enum
+from threading import RLock
+from typing import Any, Callable, Dict, List, Optional, Tuple, Union
+
+from .lightning import LightningRpc, Millisatoshi
 
 # Notice that this definition is incomplete as it only checks the
 # top-level. Arrays and Dicts could contain types that aren't encodeable. This
@@ -183,6 +184,38 @@ class Request(dict):
         self._notify("progress", d)
 
 
+@dataclass
+class Option:
+    name: str
+    default: Optional[Any]
+    description: Optional[str]
+    opt_type: str
+    value: Optional[Any]
+    multi: bool
+    deprecated: Optional[Union[bool, List[str]]]
+    dynamic: bool
+    on_change: Optional[Callable[["Plugin", str, Optional[Any]], None]]
+
+    def __getitem__(self, key):
+        """Backwards compatibility for callers who directly asked for ['value']"""
+        if key == 'value':
+            return self.value
+        raise KeyError(f"Key {key} not supported, only 'value' is")
+
+    def json(self) -> Dict[str, Any]:
+        ret = {
+            'name': self.name,
+            'description': self.description,
+            'type': self.opt_type,
+            'multi': self.multi,
+            'deprecated': self.deprecated,
+            'dynamic': self.dynamic,
+        }
+        if self.default is not None:
+            ret['default'] = self.default
+        return ret
+
+
 # If a hook call fails we need to coerce it into something the main daemon can
 # handle. Returning an error is not an option since we explicitly do not allow
 # those as a response to the calls, otherwise the only option we have is to
@@ -224,7 +257,7 @@ class Plugin(object):
             'setconfig': Method('setconfig', self._set_config, MethodType.RPCMETHOD)
         }
 
-        self.options: Dict[str, Dict[str, Any]] = {}
+        self.options: Dict[str, Option] = {}
         self.notification_topics: List[str] = []
         self.custom_msgs = custom_msgs
 
@@ -297,7 +330,7 @@ class Plugin(object):
                    category: Optional[str] = None,
                    desc: Optional[str] = None,
                    long_desc: Optional[str] = None,
-                   deprecated: Union[bool, List[str]] = None) -> None:
+                   deprecated: Optional[Union[bool, List[str]]] = None) -> None:
         """Add a plugin method to the dispatch table.
 
         The function will be expected at call time (see `_dispatch`)
@@ -388,7 +421,7 @@ class Plugin(object):
             return f
         return decorator
 
-    def add_option(self, name: str, default: Optional[str],
+    def add_option(self, name: str, default: Optional[Any],
                    description: Optional[str],
                    opt_type: str = "string",
                    deprecated: Optional[Union[bool, List[str]]] = None,
@@ -417,17 +450,17 @@ class Plugin(object):
                 'Option {} has on_change callback but is not dynamic'.format(name)
             )
 
-        self.options[name] = {
+        self.options[name] = Option(
-            'name': name,
+            name=name,
-            'default': default,
+            default=default,
-            'description': description,
+            description=description,
-            'type': opt_type,
+            opt_type=opt_type,
-            'value': None,
+            value=None,
-            'multi': multi,
+            dynamic=dynamic,
-            'deprecated': deprecated,
+            on_change=on_change,
-            "dynamic": dynamic,
+            multi=multi,
-            'on_change': on_change,
+            deprecated=deprecated if deprecated is not None else False,
-        }
+        )
 
     def add_flag_option(self, name: str, description: str,
                         deprecated: Optional[Union[bool, List[str]]] = None,
@@ -446,19 +479,19 @@ class Plugin(object):
         """
         self.notification_topics.append(topic)
 
-    def get_option(self, name: str) -> str:
+    def get_option(self, name: str) -> Optional[Any]:
         if name not in self.options:
             raise ValueError("No option with name {} registered".format(name))
 
-        if self.options[name]['value'] is not None:
+        if self.options[name].value is not None:
-            return self.options[name]['value']
+            return self.options[name].value
         else:
-            return self.options[name]['default']
+            return self.options[name].default
 
     def async_method(self, method_name: str, category: Optional[str] = None,
                      desc: Optional[str] = None,
                      long_desc: Optional[str] = None,
-                     deprecated: Union[bool, List[str]] = None) -> NoneDecoratorType:
+                     deprecated: Optional[Union[bool, List[str]]] = None) -> NoneDecoratorType:
         """Decorator to add an async plugin method to the dispatch table.
 
         Internally uses add_method.
@@ -835,16 +868,19 @@ class Plugin(object):
                 parts.append(options_header)
                 options_header = None
 
-            doc = textwrap.indent(opt['description'], prefix="    ")
+            if opt.description:
+                doc = textwrap.indent(opt.description, prefix="    ")
+            else:
+                doc = ""
 
-            if opt['multi']:
+            if opt.multi:
                 doc += "\n\n    This option can be specified multiple times"
 
             parts.append(option_tpl.format(
-                name=opt['name'],
+                name=opt.name,
                 doc=doc,
-                default=opt['default'],
+                default=opt.default,
-                typ=opt['type'],
+                typ=opt.opt_type,
             ))
 
         sys.stdout.write("".join(parts))
@@ -930,7 +966,7 @@ class Plugin(object):
                 m["long_description"] = method.long_desc
 
         manifest = {
-            'options': list({k: v for k, v in d.items() if v is not None} for d in self.options.values()),
+            'options': list(d.json() for d in self.options.values()),
             'rpcmethods': methods,
             'subscriptions': list(self.subscriptions.keys()),
             'hooks': hooks,
@@ -976,7 +1012,7 @@ class Plugin(object):
         self.rpc = LightningRpc(path)
         self.startup = verify_bool(configuration, 'startup')
         for name, value in options.items():
-            self.options[name]['value'] = value
+            self.options[name].value = value
 
         # Dispatch the plugin's init handler if any
         if self.child_init:
@@ -986,13 +1022,13 @@ class Plugin(object):
     def _set_config(self, config: str, val: Optional[Any]) -> None:
         """Called when the value of a dynamic option is changed
         """
-        cb = opt['on_change']
+        opt = self.options[config]
+        cb = opt.on_change
         if cb is not None:
             # This may throw an exception: caller will turn into error msg for user.
             cb(self, config, val)
 
-        opt = self.options[config]
+        opt.value = val
-        opt['value'] = val
 
 
 class PluginStream(object):