Inheritance in different translation units and comment resolution

[AfonsoLopo]

Hello @JhnW,
We started using devana on BMW Car IT and we needed to make some modifications to the code in order to add add functionality to the library.
The features added were :
- Inheritance resolution when the parent class is not in the same translation unit
- Documentation on classes and methods syntax abstraction
- Change some RegEx's to not depend on the formatting of the file. ( header guard resolution and template )
We wanted to contribute with these modifications to the official devana repo.

As this was a corporate development, we must add a copyright notice in the header of the modified files.
Maybe you could also add a copyright notice for yourself and your developments.
What are your thoughts on this last statement ? 😃

Please have a look in the PR : #3

Thank you for your attention,
Afonso Lopo

[JhnW]

Hi.
I will review the changes in my free time. You can expect minor changes to where the API extends (for example, documentation fields). I have an idea and a vision of how to do it fully, so I have to rethink how to implement the proposed version so as not to break the API too much in the future. The project is still in the alpha version, which allows for some freedom which, however, I would prefer not to overuse.
It's a pity that you didn't stick to the changelog generator's recommendations in your commits, but we'll fix it with merge.

As for the copyright information, we need to discuss it.
I did not anticipate the emergence of a coercive contributor so quickly. As I understand it, the legal requirements of your company assume the use of copyright in the project.

1. Are you committed to the SPDX format? Plan-text copyright info like tensorflow come into play?
2. I am thinking of a general copyright for contributors (SPD or not) and an additional file with their list. Is it good for you or would it broke the tools you use?
3. SPDX tags are used incorrectly in tests. They should never be found in the c ++ files used for testing. This disrupts the tests. These files should be treated as "blobs" (like test images without metadata etc.), not as true source code.

[heliocastro]

Hello @JhnW
I'm the OSS officer at CarIt and the one leading the compliance tooling here at BMW, so let me try help Afonso to answer the questions:

1 - Yes, every single clearance inside BMW is done in SPDX format. We are part of a large group of companies working on made somehow global the tooling used in open source. You can see here https://github.com/Open-Source-Compliance/Sharing-creates-value
So, regards SPDX, the group itself work tougether with SPDX foundation, and OpenChain.

2 - No problem with general copyrights, but then is perfect acceptable exists only on source code. Why ? because the tooling itself not scans ( or look ) a single file, but then every single line of source code, no exceptions. That's why is ok to exists only on the source code.
For example, one big OSS software that we use ( and contribute ) has their copyright in code same way, i.e.: https://github.com/eclipse/sw360/blob/master/libraries/exporters/src/main/java/org/eclipse/sw360/exporter/CSVExport.java
You can see how the license the copyright and SPDX tag are there, and the small header as required by EPL-2.0 license

About the tooling, in our pipeline we use as main tool ORT- OSS ReviewToolkit, an swiss knife of multiple components, and main scanner is ScanCode Toolkit
This one produces a complete output of entire source code, line by line, including transient dependencies as well

3 - For the point of view of compliance scanning, every single line in the repository is considered "code" as well. Even test code.
Where it differs ? We usually accept the project itself mark the tests as "test files", so we scan then, we mark as test only, and we do not interfere on the valid deployment status. But still, the code exists and is valid, is a test anyway. It appears on the final scanning, it not just goes to final document as not relevant. Just in ( remote ) case of software auditing, it need be present.

So can you explain me how the tests are broken by the string and how can we help to solve this ?

[AfonsoLopo]

Just to note, that in this commit, I've made a fix to make the syntax parsing not care if the header guards are in the top of the file or not, so actually, the tests should not break right now. ( I've tested it locally and all was fine. )

[AfonsoLopo]

I've edited the commit messages to comply with the change log generator. I'm sorry I didn't see it at first, but it should be fixed by now 😄

[JhnW]

Thank you for your comprehensive explanation.
As for licensing information in tests. The license information in the test code (python side) is fine. C ++ files used in tests are not source code as defined by devan.
Treat c ++ files as if any other format parser would treat the files you want to run on (xml parser - ml files, json parser - json files, protobuf parser - protobuf files). We do not include license information in sample json or binary files.
Adding this up makes unit testing more comprehensive. Of course it can be fixed, as @AfonsoLopo did. However, for testing purposes, it is best to have a minimal file (for example, to not test extra stuff on that rely on regular expressions).

Anyway, I had time to review the PR. A brief summary of the whole thing. If I missed any important point, please let me know.

1. Error looking for types when a type within the same module was in another file. Thanks for letting me know about this bug. By the way, I also found some other problems in the namespaces. In the last commit I think I fixed it myself. Your proposal was very good, but I did it in a different, more correct way using the generic mechanisms of the lexicon (devana heart).
2. Documentation associated with syntax elements. I plan to bite it a little differently. From the user's point of view, it will be a similar solution with the associated comment field (CodePiece or a similar type). I suggest waiting until I add the correct parsing of the comments in the code (this also applies to the preamble in the files).
3. License information in files. I understand their desire for this information, many companies prefer fork projects and then building and serving them from their own artifactory. The next time the package is released, I will add the relevant information to the source files.
4. Source file as incomplete translation unit. You're right. Currently, the source files require a lexicon containing all the types we are asking for, which is only completed with the help of the module. I have a little "hack" for typedefs. Finally, I need to develop a mechanism for healthy parsing of external types. I do not put it in a high place, with a small number of types, you can do it manually by completing the lexicon. The header code for most common C ++ libraries (std, boost) can be terrible and it is definitely not prepared for devan parsing. I gave it a not too high priority because at the API level, no one wants to use external types anyway (for example because of ensuring a stable ABI of their libraries).
5. Improving template RegEx recognition. Do you have a minimal scenario where the current parsing fails?
6. Future fixes, PR and requirements. I suggest that you first let me know about any errors or requirements before the PR comes out. Help is absolutely there (and I'm grateful) but some of the fixes, while acceptable for your use case, may have a negative impact on the future architecture (I know, the documentation lacks a technical description for future development). Once I know about the bug / requirement, I can do it myself or show you where to start. Or have absolutely no comments ;)

[AfonsoLopo]

Hello,
I've seen that you've integrated some of the proposed changes, thank you for that. ( Class Inheritance from different non-complete translation units ).
Regarding the other reported issues :

• Improvement on the RegEx's to recognize valid c++ code with different formatting. ( link to fix/change )

• Header Guards recognition break when any text is after the "#endif" expression. ( link to fix/change )

• Header Guards not recognized when we have a header in the c++ file ( link to fix/change )

• Class and Method Documentation Parsing ( link to fix/change )

Do you pretend to include those in the roadmap? Would you have a target date/release for those fixes/features ?
Best Regards,
Afonso Lopo

[JhnW]

Hello. Regarding the road map - there is no need to add it to the documentation because these are the first things that I will deal with at the moment. I am already implementing the second needed amendment regarding the header guard.
The most work will be with the documentation of classes / methods - I want to add an abstraction to the comments in the code, and then associate specific code instances with them.
Unfortunately devana is competing for access to my time with full time job and occasional freelancer jobs. I think that no later than the end of April I will release a new version with the above-mentioned fixes. I suspect it will be even sooner if I can't let you know - here or via email.

[JhnW]

Hello.
Version 0.3.0 has been released with fairly wide support for comments - parsing, code generation and file preambles.