Simplification drops essential parts of expressions

[stefan6419846]

I am using the license-expression library to feed it a large string of licenses from different package files to propose a derived package license. Unfortunately, this approach omits important licenses.

Let's consider the following example:

>>> from license_expression import get_spdx_licensing
>>> licensing = get_spdx_licensing()
>>> expression = "MIT AND BSD-3-Clause AND (MIT OR GPL-2.0-or-later OR (Apache-2.0 AND BSD-2-Clause)) AND MIT"
>>> parsed = licensing.parse(expression)
>>> str(parsed.simplify())
BSD-3-Clause AND MIT
>>> expression = "MIT AND BSD-3-Clause AND (MIT OR GPL-2.0-or-later OR Apache-2.0) AND MIT"
>>> parsed = licensing.parse(expression)
>>> str(parsed.simplify())
BSD-3-Clause AND MIT

As we can see, the whole OR sections have been removed, which would imply I already exercised a choice, which is not the case.

[stefan6419846]

It seems like the code already accounts for this by having Licensing.dedup():

license-expression/src/license_expression/__init__.py

Lines 707 to 726 in a3c00c0

	def dedup(self, expression):
	"""
	Return a deduplicated LicenseExpression given a license ``expression``
	string or LicenseExpression object.
	The deduplication process is similar to simplification but is
	specialized for working with license expressions. Simplification is
	otherwise a generic boolean operation that is not aware of the specifics
	of license expressions.
	The deduplication:
	- Does not sort the licenses of sub-expression in an expression. They
	stay in the same order as in the original expression.
	- Choices (as in "MIT or GPL") are kept as-is and not treated as
	simplifiable. This avoids droping important choice options in complex
	expressions which is never desirable.
	"""

I see some issues about this:

• This is a method of Licensing only, not of a parsed LicenseExpression (which is just an alias for the Expression from boolean.py). Given a parsed expression, I would have assumed such essential functionality to be available on the expression itself.
• The documentation is wrong. In the README, the sections about de-duplication and simplification use the same code, thus indicating that LicenseExpression.simplify() would de-duplicate properly, which is not the case.

  license-expression/README.rst

  Lines 185 to 200 in a3c00c0

  	And expression can be deduplicated, to remove duplicate license subexpressions
  	without changing the order and without consider license choices as simplifiable::
  	>>> expression2 = ' GPL-2.0 or (mit and LGPL 2.1) or bsd Or GPL-2.0 or (mit and LGPL 2.1)'
  	>>> parsed2 = licensing.parse(expression2)
  	>>> str(parsed2)
  	'GPL-2.0 OR (mit AND LGPL 2.1) OR BSD OR GPL-2.0 OR (mit AND LGPL 2.1)'
  	>>> assert str(parsed2.simplify()) == 'BSD OR GPL-2.0 OR (LGPL 2.1 AND mit)'
  	Expression can be simplified, treating them as boolean expressions::
  	>>> expression2 = ' GPL-2.0 or (mit and LGPL 2.1) or bsd Or GPL-2.0 or (mit and LGPL 2.1)'
  	>>> parsed2 = licensing.parse(expression2)
  	>>> str(parsed2)
  	'GPL-2.0 OR (mit AND LGPL 2.1) OR BSD OR GPL-2.0 OR (mit AND LGPL 2.1)'
  	>>> assert str(parsed2.simplify()) == 'BSD OR GPL-2.0 OR (LGPL 2.1 AND mit)'

• Licensing.dedup() omits the sorting usually done by LicenseExpression.simplify().
• Licensing.dedup('MIT AND (MIT AND Apache-2.0) AND Apache-2.0') does not detect that the inner expression is obsolete/the brackets could be removed.