feat(core): convert "/" in construct id to "--" and disallow tokens

[eladb]

Relax construct ID constraints to allow "/" (path seprator)
to be used in construct IDs, but convert it to "--" so it won't
collide with path strings.

It's quite rare for people to actually try to find a construct
by their ID (it's mostly "write-only") and the logical ID is eventually
mangled anyway when synthesized to CFN.

Fails if the construct ID contains a token. This won't work because
we mangle the IDs as strings when we generate the logical ID
and the construct's unique ID, and stringified tokens won't be
resolved.

Fixes #1351
Fixes #1374

---

Pull Request Checklist

Please check all boxes, including N/A items:

Testing

• Unit test and/or integration test added
• Toolkit change?: integration tests manually executed (paste output to the PR description)
• Init template change?: coordinated update of integration tests (currently maintained in a private repo).

Documentation

• README: README and/or documentation topic updated
• jsdocs: All public APIs documented

Title and description

• Change type: Title is prefixed with change type:
  • fix(module): <title> bug fix (patch)
  • feat(module): <title> feature/capability (minor)
  • chore(module): <title> won't appear in changelog
  • build(module): <title> won't appear in changelog
• Title format: Title uses lower case and doesn't end with a period
• Breaking change?: Last paragraph of description is: BREAKING CHANGE: <describe exactly what changed and how to achieve similar behavior + link to documentation/gist/issue if more details are required>
• References: Indicate issues fixed via: Fixes #xxx or Closes #xxx

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

[RomainMuller]

Why is allowing / in construct IDs necessary? That introduces a risk of collision if user code uses -- in some construct IDs (which is absolutely legal). I just don't understand how this moves the needle on the two issues that are linked...

[eladb]

@RomainMuller fully relaxing the constraints on construct IDs will make it easier to people to use any string that has the "locality" property for the ID without needing to think if this string is a valid ID, and sometimes (like #1351) those IDs are derived from user inputs. #1351 is a good example where @rix0rrr used the CloudWatch metric namespace for the construct ID, but didn't consider the option that the namespace may contain "/"s.

The risk for collision is very low, in which case users will get a very early error and they can address, so I don't think that's a real concern.

[RomainMuller]

I'm not the biggest fan of going --, but fine, I guess. The risk of a collision on -- is kinda low. I still think this should at least be documented in the escape hatch configuration (you'll have a hard time finding the constructs you want to fiddle with otherwise).

[rix0rrr]

To solve @PaulMaddox's toString() issue, do you think we should build in a substring search for 'function()' and reject that as well?

I mean, it's probably not going to be common but it has happened to a knowledgeable individual, and the check for it isn't that hard.

[eladb]

@rix0rrr not sure... People can throw all bunch of garbage into the ID that is not a "good ID", but we can't catch all of that. I can even think of a legitimate use case where function() will be part of a construct ID, so I wouldn't want to start with this cat & mouse chase.

[eladb]

@RomainMuller I don't think we need to confuse people in the documentation of the escape hatch. The docs tell you to do L2.findChild(id) (not "path"), in which case the ID will likely be something like "Resource" or some other name for L1 that is a direct child of the L2. I will add some docs to the logical IDs topic though.

[mergify]

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

• PR title type(scope): text
  • type: fix, feat, refactor go into CHANGELOG, chore is hidden
  • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
  • text: use all lower-case, do not end with a period, do not include issue refs
• PR Description
  • Rationale: describe rationale of change and approach taken
  • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
  • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
• Testing
  • Unit test added. Prefer to add a new test rather than modify existing tests
  • CLI or init templates change? Re-run/add CLI integration tests
• Documentation
  • README: update module README to describe new features
  • API docs: public APIs must be documented. Copy from official AWS docs when possible
  • Design: for significant features, follow design process