package JSON declared / name collision with CPAN

[sidey79]

In this repository, you define a package JSON:

zwiebert/tronferno-fhem/modules/tronferno/FHEM/00_TronfernoMCU.pm:package JSON;
zwiebert/tronferno-fhem/modules/tronferno/FHEM/10_Tronferno.pm:package JSON;

This breaks installing JSON from CPAN, by my new approach detecting needed and provided packages automaticly if a new docker image is build.

Is there a good reason, why you define your own JSON Package instead using the CPAN one?

[zwiebert]

This is just for adding function prototypes, which was helpful to catch coding errors. There is no other reason. I will change it.

[sidey79]

Great to here this.

Normaly, you don't want to declare prototypes at all, what you did, is something different, you declared a package with name JSON and declared subs, which have parameter prototypes.

[zwiebert]

There is a "require JSON" just before the "package JSON;". So I just extend the CPAN JSON with 2 prototypes.

I just googled: "The package statement switches the current naming context to a specified namespace (symbol table)."

And I did the same with "package main;" to provide prototypes. It makes no difference if I write the main:: prototypes at the beginning of the file (where it is main by default) or have them later after a "package main;"

[sidey79]

Yes, i understand what you mean, but i think you have another understanding of what prototype in Perl are:

Perl supports a very limited kind of compile-time argument checking using function prototyping. This can be declared in either the PROTO section or with a prototype attribute. If you declare either of

 package JSON;
 sub to_json($@);
 sub from_json($@);_

tojson() and from_json checks arguments at compile time as specified "$@". I think you mean function signatures like they are used in C header files for example. I'm not sure, if they are still experimental, but never needed them in the past.

Back to the problem. Iwas not aware, that somebody extends a existing namespace of a CPAN Module, without putting this in a seperate namespace different from the original. Currently i have no goot idea, how i can detect, that the JSON module isn't provided within your module, because that it looks like and so it is not detected as additional needed.

Not sure, if your module will run with this setup in the fhem-docker image.

[zwiebert]

I check my module with lint, and its helpful if all external functions are declared as prototypes. So I don't have to crash the server each time. I'll think about removing them in the distribution files. Maybe I even remove all the package name spaces. They may eat rescources on the server. Its no C++ unfortunally.

For a quick fix (don't want to change too much too fast) , would this work? It avoids the "package JSON", but I hope it does exactly the same:

require JSON;

sub main::AssignIoPort($;$);
sub main::AttrVal($$$);
sub main::IOWrite($@);
sub main::Log3($$$);
sub main::ReadingsVal($$$);
sub main::readingsBeginUpdate($);
sub main::readingsBulkUpdateIfChanged($$$@);
sub main::readingsEndUpdate($$);
sub main::readingsSingleUpdate($$$$;$);
sub main::Tronferno_Initialize($);
sub JSON::to_json($@);
sub JSON::from_json($@);

[sidey79]

I think this change would bring JSON module as a dependency back.

But what i do not understand, what you are trying to archive. The subs you are defining are already defined by the JSON module you are adding via require? Are you trying to implement a argument check? That won't work as expected:

See this example: https://wiki.sei.cmu.edu/confluence/plugins/servlet/mobile?contentId=88890518#content/view/88890518

Towards lint, use perlcritic, it will give you the real Painpoints.

May what you wanted is something Like

    our main::AssignIoPort;

To make this available even if FHEM ist not in the include List, which normaly it isn't

[zwiebert]

...ok, the above still works the same with the linter. So it has the same meaning as before, just written more verbose.

The linter catches any missing arguments (except for the array problem), and also nonexistant sub calls (which would work with just a declaration wihout argument-prototypes).

perl -cw -MO=Lint modules/tronferno/FHEM/00_TronfernoMCU.pm 2>&1 | grep -v 'Undefined subroutine'
Not enough arguments for JSON::to_json at modules/tronferno/FHEM/00_TronfernoMCU.pm line 406, near "$all) "
Nonexistent subroutine 'TronfernoMCU::fetch_config_all' called at modules/tronferno/FHEM/00_TronfernoMCU.pm line 374

Not useless at all. I use PerlCritiq too. But it catches different things.

[zwiebert]

perlcritic did not catch this mismatching prototype. Not sure if I could configure it differently, but I have just both Lint and perlcritic in the Makefile and as build targes in eclipse. So its easy to check with both, and they give zero warnings normally.

make 00_TronfernoMCU.critic 
perlcritic --verbose 7 modules/tronferno/FHEM/00_TronfernoMCU.pm
modules/tronferno/FHEM/00_TronfernoMCU.pm source OK

[sidey79]

Perlcritic will give you high severity warning when using prototypes as you did. This ist usually one of the first hints it gives you.

There ist a template repo available, showing how to use perlcritic within GitHub actions:

https://github.com/fhem/mod_template

It ist also capable of giving inline hints to you and many other things, but this is another story. .

[sidey79]

One last idea. If it ist not helpfull, please ignore it.

You are using require JSON instead of use JSON statement.

May this tells your linter, that the subs are not there, because it's not loaded at compile time, it's loaded at runtime.

[zwiebert]

Thanks. I "fixed" the problem for now and updated the repository.

IIRC the "require JSON" is there because I wanted to avoid crashing the server with "use JSON" at startup in case JSON was not available. This delays the crash, and the user may be able to deactivate the module via FHEMWEB or something.

[sidey79]

If you don't want a crash, use can use eval {use JSON; } instead. But, this can only occure, on perl < 5.14, because JSON:PP is a core module since this version.

I'll close this for now, may i relay later fully on "META.json" to get the needed packages, but currently only a small amount of modules provide this information, so most module won't work going this way now.

PS: when loading the JSON module, it injects to_json and from_json into the package main, so no need to delcare them in JSON package: _use JSON; # imports encode_json, decode_json, to_json and fromjson.

[zwiebert]

Thanks. JSON is often used by other FHEM packages, so I no longer see any problem in using "use". I prefer to use package prefixes like main:: and JSON:: in sub-calls. It makes it easier for me to understand what is going on in these big modules after some weeks or years.

I now found out these prototypes "headers" where actually preventing a problem from reported by the linter. So I removed them and switched to "signatures" in an experimental branch. I will remove them in the main branch later, or even use the signatures there. Not sure if its ok, to use them in a FHEM module.